[Qemu-devel] [PULL 00/15] Misc patches for QEMU 2.6.0-rc2

[Qemu-devel] [PULL 00/15] Misc patches for QEMU 2.6.0-rc2

[Paolo Bonzini]

The following changes since commit 7acbff99c6c285b3070bf0e768d56f511e2bf346:

  Update version for v2.6.0-rc1 release (2016-04-05 21:53:18 +0100)

are available in the git repository at:

  git://github.com/bonzini/qemu.git tags/for-upstream

for you to fetch changes up to c8ba3357cbd2c778d05389f3eaac00c034e4a770:

  target-i386: check for PKU even for non-writable pages (2016-04-07 13:45:19 +0200)

----------------------------------------------------------------
* NBD fixes from Alex and Eric
* Debug code bitrot from Emilio
* HPET fix from Bill
* ps2kbd fix from Hervé
* PKU fix from myself
* Coverity fixes from Gonglei
* More memory.txt update from Jiangang
* .gitignore maintenance from Changlong

----------------------------------------------------------------
Alex Bligh (1):
      nbd: Fix NBD unsupported options

Bill Paul (1):
      hw/timer: Revert "hpet: inverse polarity when pin above ISA_NUM_IRQS"

Changlong Xie (1):
      tests: ignore test-logging

Emilio G. Cota (1):
      translate-all: add missing fold of tb_ctx into tcg_ctx

Eric Blake (5):
      nbd: Return correct error for write to read-only export
      nbd: Avoid bitrot in TRACE() usage
      nbd: Improve debug traces on little-endian
      qemu-nbd: Document -x option
      nbd: Don't kill server when client requests unknown option

Gonglei (2):
      spapr: fix possible Negative array index read
      hostmem-file: fix memory leak

Hervé Poussineau (1):
      ps2kbd: default to scancode_set 2, as with KBD_CMD_RESET

Paolo Bonzini (2):
      nbd: do not hang nbd_wr_syncv if outside a coroutine and no available data
      target-i386: check for PKU even for non-writable pages

Wei Jiangang (1):
      docs: fix typo in memory.txt

 backends/hostmem-file.c |  5 +++-
 docs/memory.txt         |  8 +++----
 hw/input/ps2.c          |  2 +-
 hw/ppc/spapr.c          |  4 ++++
 hw/timer/hpet.c         | 14 ++---------
 nbd/client.c            | 63 ++++++++++++++++++++++++++++++++++++++-----------
 nbd/common.c            |  5 +++-
 nbd/nbd-internal.h      | 15 +++++++-----
 nbd/server.c            | 11 ++++++---
 qemu-nbd.c              |  1 +
 target-i386/helper.c    | 18 +++++++-------
 tests/.gitignore        |  1 +
 translate-all.c         |  3 ++-
 13 files changed, 99 insertions(+), 51 deletions(-)
-- 
2.5.5

[Qemu-devel] [PULL 01/15] ps2kbd: default to scancode_set 2, as with KBD_CMD_RESET

[Paolo Bonzini]

From: Hervé Poussineau <hpoussin@reactos.org>

This line has been added in commit ef74679a810fe6858f625b9d52b68cc3fc61eb3d with
other initializations. However, scancode set 0 doesn't exist (only 1, 2, 3).
This works well as long as operating system is resetting keyboard, or overwriting
the current scancode set with the one it wants.

This fixes IBM 40p firmware, which doesn't bother sending KBD_CMD_RESET or KBD_CMD_SCANCODE.

Signed-off-by: Hervé Poussineau <hpoussin@reactos.org>
Message-Id: <1458714100-28885-1-git-send-email-hpoussin@reactos.org>
Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
---
 hw/input/ps2.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/hw/input/ps2.c b/hw/input/ps2.c
index 58892d5..a8aa36f 100644
--- a/hw/input/ps2.c
+++ b/hw/input/ps2.c
@@ -628,7 +628,7 @@ static void ps2_kbd_reset(void *opaque)
     ps2_common_reset(&s->common);
     s->scan_enabled = 0;
     s->translate = 0;
-    s->scancode_set = 0;
+    s->scancode_set = 2;
 }
 
 static void ps2_mouse_reset(void *opaque)
-- 
2.5.5

Re: [Qemu-devel] [PULL 01/15] ps2kbd: default to scancode_set 2, as with KBD_CMD_RESET

[Peter Maydell]

On 7 April 2016 at 17:50, Paolo Bonzini <pbonzini@redhat.com> wrote:
> From: Hervé Poussineau <hpoussin@reactos.org>
>
> This line has been added in commit ef74679a810fe6858f625b9d52b68cc3fc61eb3d with
> other initializations. However, scancode set 0 doesn't exist (only 1, 2, 3).
> This works well as long as operating system is resetting keyboard, or overwriting
> the current scancode set with the one it wants.
>
> This fixes IBM 40p firmware, which doesn't bother sending KBD_CMD_RESET or KBD_CMD_SCANCODE.
>
> Signed-off-by: Hervé Poussineau <hpoussin@reactos.org>
> Message-Id: <1458714100-28885-1-git-send-email-hpoussin@reactos.org>
> Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>

Something has gone wrong with the accented character in
Hervé's signed-off-by line (you can see it is ok in the From line).

Would you mind fixing it and resending the pull request?

thanks
-- PMM

[Qemu-devel] [PULL 02/15] hw/timer: Revert "hpet: inverse polarity when pin above ISA_NUM_IRQS"

[Paolo Bonzini]

From: Bill Paul <wpaul@windriver.com>

This reverts commit 0d63b2dd31464cfccc80bbeedc24e3863fe4c895.

This change was originally intended to correct the HPET behavior
in conjunction with Linux, however the behavior that it actually creates
is not compatible with the ioapic.c implementation; it used to be
compatible with KVM's own IOAPIC but it is not anymore.

Signed-off-by: Bill Paul <wpaul@windriver.com>
CC: Paolo Bonzini <pbonzini@redhat.com>
CC: Richard Henderson <rth@twiddle.net>
CC: Eduardo Habkost <ehabkost@redhat.com>
Message-Id: <201604051558.20070.wpaul@windriver.com>
Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
---
 hw/timer/hpet.c | 14 ++------------
 1 file changed, 2 insertions(+), 12 deletions(-)

diff --git a/hw/timer/hpet.c b/hw/timer/hpet.c
index 78140e6..a2c18b3 100644
--- a/hw/timer/hpet.c
+++ b/hw/timer/hpet.c
@@ -201,12 +201,7 @@ static void update_irq(struct HPETTimer *timer, int set)
     if (!set || !timer_enabled(timer) || !hpet_enabled(timer->state)) {
         s->isr &= ~mask;
         if (!timer_fsb_route(timer)) {
-            /* fold the ICH PIRQ# pin's internal inversion logic into hpet */
-            if (route >= ISA_NUM_IRQS) {
-                qemu_irq_raise(s->irqs[route]);
-            } else {
-                qemu_irq_lower(s->irqs[route]);
-            }
+            qemu_irq_lower(s->irqs[route]);
         }
     } else if (timer_fsb_route(timer)) {
         address_space_stl_le(&address_space_memory, timer->fsb >> 32,
@@ -214,12 +209,7 @@ static void update_irq(struct HPETTimer *timer, int set)
                              NULL);
     } else if (timer->config & HPET_TN_TYPE_LEVEL) {
         s->isr |= mask;
-        /* fold the ICH PIRQ# pin's internal inversion logic into hpet */
-        if (route >= ISA_NUM_IRQS) {
-            qemu_irq_lower(s->irqs[route]);
-        } else {
-            qemu_irq_raise(s->irqs[route]);
-        }
+        qemu_irq_raise(s->irqs[route]);
     } else {
         s->isr &= ~mask;
         qemu_irq_pulse(s->irqs[route]);
-- 
2.5.5

[Qemu-devel] [PULL 03/15] docs: fix typo in memory.txt

[Paolo Bonzini]

From: Wei Jiangang <weijg.fnst@cn.fujitsu.com>

The space between 7000 and 8000 is too wide by 1 character.
Also correct the range of vga-window example 0xa0000-0xbffff.

Signed-off-by: Wei Jiangang <weijg.fnst@cn.fujitsu.com>
Message-Id: <1458639954-9980-1-git-send-email-weijg.fnst@cn.fujitsu.com>
Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
---
 docs/memory.txt | 8 ++++----
 1 file changed, 4 insertions(+), 4 deletions(-)

diff --git a/docs/memory.txt b/docs/memory.txt
index f9272ca..431d9ca 100644
--- a/docs/memory.txt
+++ b/docs/memory.txt
@@ -185,9 +185,9 @@ an MMIO region mapped at 0x0, size 0x6000, priority 1. B currently has two
 of its own subregions: D of size 0x1000 at offset 0 and E of size 0x1000 at
 offset 0x2000. As a diagram:
 
-        0      1000   2000   3000   4000   5000   6000   7000    8000
-        |------|------|------|------|------|------|------|-------|
-  A:    [                                                       ]
+        0      1000   2000   3000   4000   5000   6000   7000   8000
+        |------|------|------|------|------|------|------|------|
+  A:    [                                                      ]
   C:    [CCCCCCCCCCCCCCCCCCCCCCCCCCCCCCCCCCCCCCCC]
   B:                  [                          ]
   D:                  [DDDDD]
@@ -247,7 +247,7 @@ system_memory: container@0-2^48-1
  |
  +---- himem: alias@0x100000000-0x11fffffff ---> #ram (0xe0000000-0xffffffff)
  |
- +---- vga-window: alias@0xa0000-0xbfffff ---> #pci (0xa0000-0xbffff)
+ +---- vga-window: alias@0xa0000-0xbffff ---> #pci (0xa0000-0xbffff)
  |      (prio 1)
  |
  +---- pci-hole: alias@0xe0000000-0xffffffff ---> #pci (0xe0000000-0xffffffff)
-- 
2.5.5

[Qemu-devel] [PULL 04/15] nbd: Return correct error for write to read-only export

[Paolo Bonzini]

From: Eric Blake <eblake@redhat.com>

The NBD Protocol requires that servers should send EPERM for
attempts to write (or trim) a read-only export.  We were
correct for TRIM (blk_co_discard() gave EPERM); but were
manually setting EROFS which then got mapped to EINVAL over
the wire on writes.

Signed-off-by: Eric Blake <eblake@redhat.com>
Message-Id: <1459913704-19949-2-git-send-email-eblake@redhat.com>
Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
---
 nbd/server.c | 1 +
 1 file changed, 1 insertion(+)

diff --git a/nbd/server.c b/nbd/server.c
index b95571b..98e3957 100644
--- a/nbd/server.c
+++ b/nbd/server.c
@@ -26,6 +26,7 @@ static int system_errno_to_nbd_errno(int err)
     case 0:
         return NBD_SUCCESS;
     case EPERM:
+    case EROFS:
         return NBD_EPERM;
     case EIO:
         return NBD_EIO;
-- 
2.5.5

[Qemu-devel] [PULL 05/15] nbd: Avoid bitrot in TRACE() usage

[Paolo Bonzini]

From: Eric Blake <eblake@redhat.com>

The compiler is smart enough to optimize out 'if (0)', but won't
type-check our printfs if they are hidden behind #if.

Signed-off-by: Eric Blake <eblake@redhat.com>
Message-Id: <1459913704-19949-3-git-send-email-eblake@redhat.com>
Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
---
 nbd/nbd-internal.h | 15 +++++++++------
 1 file changed, 9 insertions(+), 6 deletions(-)

diff --git a/nbd/nbd-internal.h b/nbd/nbd-internal.h
index d09b4ee..3791535 100644
--- a/nbd/nbd-internal.h
+++ b/nbd/nbd-internal.h
@@ -33,18 +33,21 @@
 /* #define DEBUG_NBD */
 
 #ifdef DEBUG_NBD
-#define TRACE(msg, ...) do { \
-    LOG(msg, ## __VA_ARGS__); \
-} while(0)
+#define DEBUG_NBD_PRINT 1
 #else
-#define TRACE(msg, ...) \
-    do { } while (0)
+#define DEBUG_NBD_PRINT 0
 #endif
 
+#define TRACE(msg, ...) do { \
+    if (DEBUG_NBD_PRINT) { \
+        LOG(msg, ## __VA_ARGS__); \
+    } \
+} while (0)
+
 #define LOG(msg, ...) do { \
     fprintf(stderr, "%s:%s():L%d: " msg "\n", \
             __FILE__, __FUNCTION__, __LINE__, ## __VA_ARGS__); \
-} while(0)
+} while (0)
 
 /* This is all part of the "official" NBD API.
  *
-- 
2.5.5

[Qemu-devel] [PULL 06/15] nbd: Improve debug traces on little-endian

[Paolo Bonzini]

From: Eric Blake <eblake@redhat.com>

Print debug tracing messages while data is still in native
ordering, rather than after we've potentially swapped it into
network order for transmission.  Also, it's nice if the server
mentions what it is replying, to correlate it to with what the
client says it is receiving.

Signed-off-by: Eric Blake <eblake@redhat.com>
Message-Id: <1459913704-19949-4-git-send-email-eblake@redhat.com>
Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
---
 nbd/client.c | 8 ++++----
 nbd/server.c | 5 +++--
 2 files changed, 7 insertions(+), 6 deletions(-)

diff --git a/nbd/client.c b/nbd/client.c
index d9b7a9b..1593cd6 100644
--- a/nbd/client.c
+++ b/nbd/client.c
@@ -628,16 +628,16 @@ ssize_t nbd_send_request(QIOChannel *ioc, struct nbd_request *request)
     uint8_t buf[NBD_REQUEST_SIZE];
     ssize_t ret;
 
+    TRACE("Sending request to server: "
+          "{ .from = %" PRIu64", .len = %u, .handle = %" PRIu64", .type=%i}",
+          request->from, request->len, request->handle, request->type);
+
     cpu_to_be32w((uint32_t*)buf, NBD_REQUEST_MAGIC);
     cpu_to_be32w((uint32_t*)(buf + 4), request->type);
     cpu_to_be64w((uint64_t*)(buf + 8), request->handle);
     cpu_to_be64w((uint64_t*)(buf + 16), request->from);
     cpu_to_be32w((uint32_t*)(buf + 24), request->len);
 
-    TRACE("Sending request to server: "
-          "{ .from = %" PRIu64", .len = %u, .handle = %" PRIu64", .type=%i}",
-          request->from, request->len, request->handle, request->type);
-
     ret = write_sync(ioc, buf, sizeof(buf));
     if (ret < 0) {
         return ret;
diff --git a/nbd/server.c b/nbd/server.c
index 98e3957..6d9c15a 100644
--- a/nbd/server.c
+++ b/nbd/server.c
@@ -656,6 +656,9 @@ static ssize_t nbd_send_reply(QIOChannel *ioc, struct nbd_reply *reply)
 
     reply->error = system_errno_to_nbd_errno(reply->error);
 
+    TRACE("Sending response to client: { .error = %d, handle = %" PRIu64 " }",
+          reply->error, reply->handle);
+
     /* Reply
        [ 0 ..  3]    magic   (NBD_REPLY_MAGIC)
        [ 4 ..  7]    error   (0 == no error)
@@ -665,8 +668,6 @@ static ssize_t nbd_send_reply(QIOChannel *ioc, struct nbd_reply *reply)
     stl_be_p(buf + 4, reply->error);
     stq_be_p(buf + 8, reply->handle);
 
-    TRACE("Sending response to client");
-
     ret = write_sync(ioc, buf, sizeof(buf));
     if (ret < 0) {
         return ret;
-- 
2.5.5

[Qemu-devel] [PULL 07/15] qemu-nbd: Document -x option

[Paolo Bonzini]

From: Eric Blake <eblake@redhat.com>

Commit 3d4b2f9c added -x to force qemu-nbd to use new-style
negotiation, but while it documented it in the man page, it
omitted docs in the --help output.

Signed-off-by: Eric Blake <eblake@redhat.com>
Message-Id: <1459908128-11925-1-git-send-email-eblake@redhat.com>
Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
---
 qemu-nbd.c | 1 +
 1 file changed, 1 insertion(+)

diff --git a/qemu-nbd.c b/qemu-nbd.c
index ca4a724..c2e4d3f 100644
--- a/qemu-nbd.c
+++ b/qemu-nbd.c
@@ -75,6 +75,7 @@ static void usage(const char *name)
 "  -e, --shared=NUM          device can be shared by NUM clients (default '1')\n"
 "  -t, --persistent          don't exit on the last connection\n"
 "  -v, --verbose             display extra debugging information\n"
+"  -x, --export-name=NAME    expose export by name\n"
 "\n"
 "Exposing part of the image:\n"
 "  -o, --offset=OFFSET       offset into the image\n"
-- 
2.5.5

[Qemu-devel] [PULL 08/15] nbd: Fix NBD unsupported options

[Paolo Bonzini]

From: Alex Bligh <alex@alex.org.uk>

nbd-client.c currently fails to handle unsupported options properly.
If during option haggling the server finds an option that is
unsupported, it returns an NBD_REP_ERR_UNSUP reply.

According to nbd's proto.md, the format for such a reply
should be:

  S: 64 bits, 0x3e889045565a9 (magic number for replies)
  S: 32 bits, the option as sent by the client to which this is a reply
  S: 32 bits, reply type (e.g., NBD_REP_ACK for successful completion,
     or NBD_REP_ERR_UNSUP to mark use of an option not known by this server
  S: 32 bits, length of the reply. This may be zero for some replies,
     in which case the next field is not sent
  S: any data as required by the reply (e.g., an export name in the case
     of NBD_REP_SERVER, or optional UTF-8 message for NBD_REP_ERR_*)

However, in nbd-client.c, the reply type was being read, and if it
contained an error, it was bailing out and issuing the next option
request without first reading the length. This meant that the
next option / handshake read had an extra 4 or more bytes of data in it.
In practice, this makes Qemu incompatible with servers that do not
support NBD_OPT_LIST.

To verify this isn't an error in the specification or my reading of
it, replies are sent by the reference implementation here:
 https://github.com/yoe/nbd/blob/66dfb35/nbd-server.c#L1232
and as is evident it always sends a 'datasize' (aka length) 32 bit
word. Unsupported elements are replied to here:
 https://github.com/yoe/nbd/blob/66dfb35/nbd-server.c#L1371

Signed-off-by: Alex Bligh <alex@alex.org.uk>
Message-Id: <1459882500-24316-1-git-send-email-alex@alex.org.uk>
[rework to ALWAYS consume an optional UTF-8 message from the server]
Signed-off-by: Eric Blake <eblake@redhat.com>
Message-Id: <1459961962-18771-1-git-send-email-eblake@redhat.com>
Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
---
 nbd/client.c | 55 +++++++++++++++++++++++++++++++++++++++++++++----------
 1 file changed, 45 insertions(+), 10 deletions(-)

diff --git a/nbd/client.c b/nbd/client.c
index 1593cd6..6777e58 100644
--- a/nbd/client.c
+++ b/nbd/client.c
@@ -73,16 +73,46 @@ static QTAILQ_HEAD(, NBDExport) exports = QTAILQ_HEAD_INITIALIZER(exports);
 */
 
 
-static int nbd_handle_reply_err(uint32_t opt, uint32_t type, Error **errp)
+/* If type represents success, return 1 without further action.
+ * If type represents an error reply, consume the rest of the packet on ioc.
+ * Then return 0 for unsupported (so the client can fall back to
+ * other approaches), or -1 with errp set for other errors.
+ */
+static int nbd_handle_reply_err(QIOChannel *ioc, uint32_t opt, uint32_t type,
+                                Error **errp)
 {
+    uint32_t len;
+    char *msg = NULL;
+    int result = -1;
+
     if (!(type & (1 << 31))) {
-        return 0;
+        return 1;
+    }
+
+    if (read_sync(ioc, &len, sizeof(len)) != sizeof(len)) {
+        error_setg(errp, "failed to read option length");
+        return -1;
+    }
+    len = be32_to_cpu(len);
+    if (len) {
+        if (len > NBD_MAX_BUFFER_SIZE) {
+            error_setg(errp, "server's error message is too long");
+            goto cleanup;
+        }
+        msg = g_malloc(len + 1);
+        if (read_sync(ioc, msg, len) != len) {
+            error_setg(errp, "failed to read option error message");
+            goto cleanup;
+        }
+        msg[len] = '\0';
     }
 
     switch (type) {
     case NBD_REP_ERR_UNSUP:
-        error_setg(errp, "Unsupported option type %x", opt);
-        break;
+        TRACE("server doesn't understand request %d, attempting fallback",
+              opt);
+        result = 0;
+        goto cleanup;
 
     case NBD_REP_ERR_POLICY:
         error_setg(errp, "Denied by server for option %x", opt);
@@ -101,7 +131,13 @@ static int nbd_handle_reply_err(uint32_t opt, uint32_t type, Error **errp)
         break;
     }
 
-    return -1;
+    if (msg) {
+        error_append_hint(errp, "%s\n", msg);
+    }
+
+ cleanup:
+    g_free(msg);
+    return result;
 }
 
 static int nbd_receive_list(QIOChannel *ioc, char **name, Error **errp)
@@ -111,6 +147,7 @@ static int nbd_receive_list(QIOChannel *ioc, char **name, Error **errp)
     uint32_t type;
     uint32_t len;
     uint32_t namelen;
+    int error;
 
     *name = NULL;
     if (read_sync(ioc, &magic, sizeof(magic)) != sizeof(magic)) {
@@ -138,11 +175,9 @@ static int nbd_receive_list(QIOChannel *ioc, char **name, Error **errp)
         return -1;
     }
     type = be32_to_cpu(type);
-    if (type == NBD_REP_ERR_UNSUP) {
-        return 0;
-    }
-    if (nbd_handle_reply_err(opt, type, errp) < 0) {
-        return -1;
+    error = nbd_handle_reply_err(ioc, opt, type, errp);
+    if (error <= 0) {
+        return error;
     }
 
     if (read_sync(ioc, &len, sizeof(len)) != sizeof(len)) {
-- 
2.5.5

[Qemu-devel] [PULL 09/15] nbd: Don't kill server when client requests unknown option

[Paolo Bonzini]

From: Eric Blake <eblake@redhat.com>

nbd-server.c currently fails to handle unsupported options properly.
If during option haggling the client sends an unknown request, the
server kills the connection instead of letting the client try to
fall back to something older.  This is precisely what advertising
NBD_FLAG_FIXED_NEWSTYLE was supposed to fix.

Signed-off-by: Eric Blake <eblake@redhat.com>
Message-Id: <1459982918-32229-1-git-send-email-eblake@redhat.com>
Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
---
 nbd/server.c | 5 ++++-
 1 file changed, 4 insertions(+), 1 deletion(-)

diff --git a/nbd/server.c b/nbd/server.c
index 6d9c15a..2a4dd10 100644
--- a/nbd/server.c
+++ b/nbd/server.c
@@ -483,9 +483,12 @@ static int nbd_negotiate_options(NBDClient *client)
                 return -EINVAL;
             default:
                 TRACE("Unsupported option 0x%x", clientflags);
+                if (nbd_negotiate_drop_sync(client->ioc, length) != length) {
+                    return -EIO;
+                }
                 nbd_negotiate_send_rep(client->ioc, NBD_REP_ERR_UNSUP,
                                        clientflags);
-                return -EINVAL;
+                break;
             }
         } else {
             /*
-- 
2.5.5

Re: [Qemu-devel] [PULL 09/15] nbd: Don't kill server when client requests unknown option

[Eric Blake]

[-- Attachment #1: Type: text/plain, Size: 768 bytes --]

On 04/07/2016 10:50 AM, Paolo Bonzini wrote:
> From: Eric Blake <eblake@redhat.com>
> 
> nbd-server.c currently fails to handle unsupported options properly.
> If during option haggling the client sends an unknown request, the
> server kills the connection instead of letting the client try to
> fall back to something older.  This is precisely what advertising
> NBD_FLAG_FIXED_NEWSTYLE was supposed to fix.
> 

I found a couple more in our server's handling of NBD_OPT_STARTTLS; now
that you are probably going to resend the pull request, you may want to
include that one.
https://lists.gnu.org/archive/html/qemu-devel/2016-04/msg01299.html

-- 
Eric Blake   eblake redhat com    +1-919-301-3266
Libvirt virtualization library http://libvirt.org


[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 604 bytes --]

[Qemu-devel] [PULL 10/15] nbd: do not hang nbd_wr_syncv if outside a coroutine and no available data

[Paolo Bonzini]

Until commit 1c778ef7 ("nbd: convert to using I/O channels for actual
socket I/O", 2016-02-16), nbd_wr_sync returned -EAGAIN this scenario.
nbd_reply_ready required these semantics because it has two conflicting
requirements:

1) if a reply can be received on the socket, nbd_reply_ready needs
to read the header outside coroutine context to identify _which_
coroutine to enter to process the rest of the reply

2) on the other hand, nbd_reply_ready can find a false positive if
another thread (e.g. a VCPU thread running aio_poll) sneaks in and
calls nbd_reply_ready too.  In this case nbd_reply_ready does nothing
and expects nbd_wr_syncv to return -EAGAIN.

Currently, the solution to the first requirement is to wait in the very
rare case of a read() that doesn't retrieve the reply header in its
entirety; this is what nbd_wr_syncv does by calling qio_channel_wait().
However, the unconditional call to qio_channel_wait() breaks the second
requirement.  To fix this, the patch makes nbd_wr_syncv return -EAGAIN
if done is zero, similar to the code before commit 1c778ef7.

This is okay because NBD client-side negotiation is the only other case
that calls nbd_wr_syncv outside a coroutine, and it places the socket
in blocking mode.  On the other hand, it is a bit unpleasant to put
this in nbd_wr_syncv(), because the function is used by both client
and server.

The full fix would be to add a counter to NbdClientSession for how
many bytes have been filled in s->reply.  Then a reply can be filled
by multiple separate invocations of nbd_reply_ready and the
qio_channel_wait() call can be removed completely.  Something to
consider for 2.7...

Reported-by: Changlong Xie <xiecl.fnst@cn.fujitsu.com>
Cc: Daniel P. Berrange <berrange@redhat.com>
Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
---
 nbd/common.c | 5 ++++-
 1 file changed, 4 insertions(+), 1 deletion(-)

diff --git a/nbd/common.c b/nbd/common.c
index a44718c..8ddb2dd 100644
--- a/nbd/common.c
+++ b/nbd/common.c
@@ -50,9 +50,12 @@ ssize_t nbd_wr_syncv(QIOChannel *ioc,
                  * qio_channel_yield() that works with AIO contexts
                  * and consider using that in this branch */
                 qemu_coroutine_yield();
-            } else {
+            } else if (done) {
+                /* XXX this is needed by nbd_reply_ready.  */
                 qio_channel_wait(ioc,
                                  do_read ? G_IO_IN : G_IO_OUT);
+            } else {
+                return -EAGAIN;
             }
             continue;
         }
-- 
2.5.5

[Qemu-devel] [PULL 11/15] spapr: fix possible Negative array index read

[Paolo Bonzini]

From: Gonglei <arei.gonglei@huawei.com>

fix CID 1351391.

Signed-off-by: Gonglei <arei.gonglei@huawei.com>
Message-Id: <1456998223-12356-6-git-send-email-arei.gonglei@huawei.com>
Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
---
 hw/ppc/spapr.c | 4 ++++
 1 file changed, 4 insertions(+)

diff --git a/hw/ppc/spapr.c b/hw/ppc/spapr.c
index e7be21e..feaab08 100644
--- a/hw/ppc/spapr.c
+++ b/hw/ppc/spapr.c
@@ -2208,6 +2208,10 @@ static void spapr_machine_device_plug(HotplugHandler *hotplug_dev,
         if (*errp) {
             return;
         }
+        if (node < 0 || node >= MAX_NODES) {
+            error_setg(errp, "Invaild node %d", node);
+            return;
+        }
 
         /*
          * Currently PowerPC kernel doesn't allow hot-adding memory to
-- 
2.5.5

[Qemu-devel] [PULL 12/15] hostmem-file: fix memory leak

[Paolo Bonzini]

From: Gonglei <arei.gonglei@huawei.com>

Signed-off-by: Gonglei <arei.gonglei@huawei.com>
Message-Id: <1456998223-12356-5-git-send-email-arei.gonglei@huawei.com>
Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
---
 backends/hostmem-file.c | 5 ++++-
 1 file changed, 4 insertions(+), 1 deletion(-)

diff --git a/backends/hostmem-file.c b/backends/hostmem-file.c
index c70f268..b17a1f1 100644
--- a/backends/hostmem-file.c
+++ b/backends/hostmem-file.c
@@ -52,11 +52,14 @@ file_backend_memory_alloc(HostMemoryBackend *backend, Error **errp)
     error_setg(errp, "-mem-path not supported on this host");
 #else
     if (!memory_region_size(&backend->mr)) {
+        gchar *path;
         backend->force_prealloc = mem_prealloc;
+        path = object_get_canonical_path(OBJECT(backend));
         memory_region_init_ram_from_file(&backend->mr, OBJECT(backend),
-                                 object_get_canonical_path(OBJECT(backend)),
+                                 path,
                                  backend->size, fb->share,
                                  fb->mem_path, errp);
+        g_free(path);
     }
 #endif
 }
-- 
2.5.5

[Qemu-devel] [PULL 13/15] translate-all: add missing fold of tb_ctx into tcg_ctx

[Paolo Bonzini]

From: "Emilio G. Cota" <cota@braap.org>

Since 5e5f07e08 "TCG: Move translation block variables
to new context inside tcg_ctx: tb_ctx" on Feb 1 2013, compilation
of usermode + TB_DEBUG_CHECK has been broken. Fix it.

Signed-off-by: Emilio G. Cota <cota@braap.org>
Message-Id: <1459834253-8291-2-git-send-email-cota@braap.org>
Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
---
 translate-all.c | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/translate-all.c b/translate-all.c
index b4df1ec..8329ea6 100644
--- a/translate-all.c
+++ b/translate-all.c
@@ -861,7 +861,8 @@ static void tb_invalidate_check(target_ulong address)
 
     address &= TARGET_PAGE_MASK;
     for (i = 0; i < CODE_GEN_PHYS_HASH_SIZE; i++) {
-        for (tb = tb_ctx.tb_phys_hash[i]; tb != NULL; tb = tb->phys_hash_next) {
+        for (tb = tcg_ctx.tb_ctx.tb_phys_hash[i]; tb != NULL;
+             tb = tb->phys_hash_next) {
             if (!(address + TARGET_PAGE_SIZE <= tb->pc ||
                   address >= tb->pc + tb->size)) {
                 printf("ERROR invalidate: address=" TARGET_FMT_lx
-- 
2.5.5

[Qemu-devel] [PULL 14/15] tests: ignore test-logging

[Paolo Bonzini]

From: Changlong Xie <xiecl.fnst@cn.fujitsu.com>

Commit 3514552e added a new test, but did not mark it for
exclusion in .gitignore.

Signed-off-by: Changlong Xie <xiecl.fnst@cn.fujitsu.com>
Reviewed-by: Eric Blake <eblake@redhat.com>
Message-Id: <1459903756-30672-1-git-send-email-xiecl.fnst@cn.fujitsu.com>
Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
---
 tests/.gitignore | 1 +
 1 file changed, 1 insertion(+)

diff --git a/tests/.gitignore b/tests/.gitignore
index b7bf13e..9eed229 100644
--- a/tests/.gitignore
+++ b/tests/.gitignore
@@ -39,6 +39,7 @@ test-io-channel-file.txt
 test-io-channel-socket
 test-io-channel-tls
 test-io-task
+test-logging
 test-mul64
 test-opts-visitor
 test-qapi-event.[ch]
-- 
2.5.5

[Qemu-devel] [PULL 15/15] target-i386: check for PKU even for non-writable pages

[Paolo Bonzini]

Xiao Guangrong ran kvm-unit-tests on an actual machine with PKU and
found that it fails:

test pte.p pte.user pde.p pde.user pde.a pde.pse pkru.wd pkey=1 user write efer.nx cr4.pke: FAIL: error code 27 expected 7
Dump mapping: address: 0x123400000000
------L4: 2ebe007
------L3: 2ebf007
------L2: 8000000020000a5

(All failures are combinations of "pde.user pde.p pkru.wd pkey=1",
plus either "pde.pse" or "pte.p pte.user", plus one of "user cr0.wp",
"cr0.wp" or "user", plus unimportant bits such as accessed/dirty or
efer.nx).

So PFEC.PKEY is set even if the ordinary check failed (which it did
because pde.w is zero).  Adjust QEMU to match behavior of silicon.

Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
---
 target-i386/helper.c | 18 ++++++++++--------
 1 file changed, 10 insertions(+), 8 deletions(-)

diff --git a/target-i386/helper.c b/target-i386/helper.c
index 5755839..bf3e762 100644
--- a/target-i386/helper.c
+++ b/target-i386/helper.c
@@ -919,29 +919,31 @@ do_check_protect_pse36:
          !((env->cr[4] & CR4_SMEP_MASK) && (ptep & PG_USER_MASK)))) {
         prot |= PAGE_EXEC;
     }
-
-    if ((prot & (1 << is_write1)) == 0) {
-        goto do_fault_protect;
-    }
-
     if ((env->cr[4] & CR4_PKE_MASK) && (env->hflags & HF_LMA_MASK) &&
         (ptep & PG_USER_MASK) && env->pkru) {
         uint32_t pk = (pte & PG_PKRU_MASK) >> PG_PKRU_BIT;
         uint32_t pkru_ad = (env->pkru >> pk * 2) & 1;
         uint32_t pkru_wd = (env->pkru >> pk * 2) & 2;
+        uint32_t pkru_prot = PAGE_READ | PAGE_WRITE | PAGE_EXEC;
 
         if (pkru_ad) {
-            prot &= ~(PAGE_READ | PAGE_WRITE);
+            pkru_prot &= ~(PAGE_READ | PAGE_WRITE);
         } else if (pkru_wd && (is_user || env->cr[0] & CR0_WP_MASK)) {
-            prot &= ~PAGE_WRITE;
+            pkru_prot &= ~PAGE_WRITE;
         }
-        if ((prot & (1 << is_write1)) == 0) {
+
+        prot &= pkru_prot;
+        if ((pkru_prot & (1 << is_write1)) == 0) {
             assert(is_write1 != 2);
             error_code |= PG_ERROR_PK_MASK;
             goto do_fault_protect;
         }
     }
 
+    if ((prot & (1 << is_write1)) == 0) {
+        goto do_fault_protect;
+    }
+
     /* yes, it can! */
     is_dirty = is_write && !(pte & PG_DIRTY_MASK);
     if (!(pte & PG_ACCESSED_MASK) || is_dirty) {
-- 
2.5.5