[Qemu-devel] [PULL 0/8] pci, virtio bugfixes for 2.3

[Qemu-devel] [PULL 0/8] pci, virtio bugfixes for 2.3

[Michael S. Tsirkin]

The following changes since commit 18bf9e2f379334306530cbfd44218748eceaf67d:

  virtio-scsi: remove empty wrapper for cmd (2015-03-11 18:24:30 +0100)

are available in the git repository at:

  git://git.kernel.org/pub/scm/virt/kvm/mst/qemu.git tags/for_upstream

for you to fetch changes up to ce394947a75296fc10f1676932473e92aa8be11a:

  pcie_aer: fix comment to match pcie spec (2015-03-18 12:48:21 +0100)

----------------------------------------------------------------
pci, virtio bugfixes for 2.3

Just a bunch of bugfixes. Should be nothing remarkable here.

Signed-off-by: Michael S. Tsirkin <mst@redhat.com>

----------------------------------------------------------------
Chen Fan (5):
      pcie: correct mistaken register bit for End-End TLP Prefix Blocking
      aer: fix wrong check on expose aer tlp prefix log
      pcie_aer: fix typos in pcie_aer_inject_error comment
      aer: fix a wrong init PCI_ERR_COR_STATUS w1cmask type register
      pci: fix several trivial typos in comment

Jason Wang (1):
      virtio: validate the existence of handle_output before calling it

Michael S. Tsirkin (1):
      pcie_aer: fix comment to match pcie spec

Stefan Weil (1):
      virtio: Fix memory leaks reported by Coverity

 include/hw/pci/pci.h       |  2 +-
 include/hw/pci/pcie_aer.h  |  2 +-
 include/hw/pci/pcie_regs.h |  2 +-
 hw/9pfs/virtio-9p-local.c  | 28 ++++++++--------------------
 hw/pci/pcie.c              |  2 +-
 hw/pci/pcie_aer.c          | 12 ++++++------
 hw/virtio/virtio.c         |  3 ++-
 7 files changed, 20 insertions(+), 31 deletions(-)

[Qemu-devel] [PULL 1/8] virtio: validate the existence of handle_output before calling it

[Michael S. Tsirkin]

From: Jason Wang <jasowang@redhat.com>

We don't validate the existence of handle_output which may let a buggy
guest to trigger a SIGSEV easily. E.g:

1) write 10 to queue_sel to a virtio net device with only 1 queue
2) setup an arbitrary pfn
3) then notify queue 10

Fixing this by validating the existence of handle_output before.

Cc: qemu-stable@nongnu.org
Cc: Michael S. Tsirkin <mst@redhat.com>
Signed-off-by: Jason Wang <jasowang@redhat.com>
Reviewed-by: Michael S. Tsirkin <mst@redhat.com>
Signed-off-by: Michael S. Tsirkin <mst@redhat.com>
Reviewed-by: Don Koch <dkoch@verizon.com>
Reviewed-by: Fam Zheng <famz@redhat.com>
---
 hw/virtio/virtio.c | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/hw/virtio/virtio.c b/hw/virtio/virtio.c
index 3c6e430..17c1260 100644
--- a/hw/virtio/virtio.c
+++ b/hw/virtio/virtio.c
@@ -759,8 +759,9 @@ void virtio_queue_set_align(VirtIODevice *vdev, int n, int align)
 
 void virtio_queue_notify_vq(VirtQueue *vq)
 {
-    if (vq->vring.desc) {
+    if (vq->vring.desc && vq->handle_output) {
         VirtIODevice *vdev = vq->vdev;
+
         trace_virtio_queue_notify(vdev, vq - vdev->vq, vq);
         vq->handle_output(vdev, vq);
     }
-- 
MST

[Qemu-devel] [PULL 2/8] virtio: Fix memory leaks reported by Coverity

[Michael S. Tsirkin]

From: Stefan Weil <sw@weilnetz.de>

All four leaks are similar, so fix them in one patch.

Signed-off-by: Stefan Weil <sw@weilnetz.de>
Reviewed-by: Michael S. Tsirkin <mst@redhat.com>
Signed-off-by: Michael S. Tsirkin <mst@redhat.com>
---
 hw/9pfs/virtio-9p-local.c | 28 ++++++++--------------------
 1 file changed, 8 insertions(+), 20 deletions(-)

diff --git a/hw/9pfs/virtio-9p-local.c b/hw/9pfs/virtio-9p-local.c
index d05c917..d66abcd 100644
--- a/hw/9pfs/virtio-9p-local.c
+++ b/hw/9pfs/virtio-9p-local.c
@@ -488,7 +488,7 @@ static int local_mknod(FsContext *fs_ctx, V9fsPath *dir_path,
     int err = -1;
     int serrno = 0;
     V9fsString fullname;
-    char *buffer;
+    char *buffer = NULL;
 
     v9fs_string_init(&fullname);
     v9fs_string_sprintf(&fullname, "%s/%s", dir_path->data, name);
@@ -499,7 +499,6 @@ static int local_mknod(FsContext *fs_ctx, V9fsPath *dir_path,
         buffer = rpath(fs_ctx, path);
         err = mknod(buffer, SM_LOCAL_MODE_BITS|S_IFREG, 0);
         if (err == -1) {
-            g_free(buffer);
             goto out;
         }
         err = local_set_xattr(buffer, credp);
@@ -512,7 +511,6 @@ static int local_mknod(FsContext *fs_ctx, V9fsPath *dir_path,
         buffer = rpath(fs_ctx, path);
         err = mknod(buffer, SM_LOCAL_MODE_BITS|S_IFREG, 0);
         if (err == -1) {
-            g_free(buffer);
             goto out;
         }
         err = local_set_mapped_file_attr(fs_ctx, path, credp);
@@ -525,7 +523,6 @@ static int local_mknod(FsContext *fs_ctx, V9fsPath *dir_path,
         buffer = rpath(fs_ctx, path);
         err = mknod(buffer, credp->fc_mode, credp->fc_rdev);
         if (err == -1) {
-            g_free(buffer);
             goto out;
         }
         err = local_post_create_passthrough(fs_ctx, path, credp);
@@ -539,8 +536,8 @@ static int local_mknod(FsContext *fs_ctx, V9fsPath *dir_path,
 err_end:
     remove(buffer);
     errno = serrno;
-    g_free(buffer);
 out:
+    g_free(buffer);
     v9fs_string_free(&fullname);
     return err;
 }
@@ -552,7 +549,7 @@ static int local_mkdir(FsContext *fs_ctx, V9fsPath *dir_path,
     int err = -1;
     int serrno = 0;
     V9fsString fullname;
-    char *buffer;
+    char *buffer = NULL;
 
     v9fs_string_init(&fullname);
     v9fs_string_sprintf(&fullname, "%s/%s", dir_path->data, name);
@@ -563,7 +560,6 @@ static int local_mkdir(FsContext *fs_ctx, V9fsPath *dir_path,
         buffer = rpath(fs_ctx, path);
         err = mkdir(buffer, SM_LOCAL_DIR_MODE_BITS);
         if (err == -1) {
-            g_free(buffer);
             goto out;
         }
         credp->fc_mode = credp->fc_mode|S_IFDIR;
@@ -576,7 +572,6 @@ static int local_mkdir(FsContext *fs_ctx, V9fsPath *dir_path,
         buffer = rpath(fs_ctx, path);
         err = mkdir(buffer, SM_LOCAL_DIR_MODE_BITS);
         if (err == -1) {
-            g_free(buffer);
             goto out;
         }
         credp->fc_mode = credp->fc_mode|S_IFDIR;
@@ -590,7 +585,6 @@ static int local_mkdir(FsContext *fs_ctx, V9fsPath *dir_path,
         buffer = rpath(fs_ctx, path);
         err = mkdir(buffer, credp->fc_mode);
         if (err == -1) {
-            g_free(buffer);
             goto out;
         }
         err = local_post_create_passthrough(fs_ctx, path, credp);
@@ -604,8 +598,8 @@ static int local_mkdir(FsContext *fs_ctx, V9fsPath *dir_path,
 err_end:
     remove(buffer);
     errno = serrno;
-    g_free(buffer);
 out:
+    g_free(buffer);
     v9fs_string_free(&fullname);
     return err;
 }
@@ -659,7 +653,7 @@ static int local_open2(FsContext *fs_ctx, V9fsPath *dir_path, const char *name,
     int err = -1;
     int serrno = 0;
     V9fsString fullname;
-    char *buffer;
+    char *buffer = NULL;
 
     /*
      * Mark all the open to not follow symlinks
@@ -675,7 +669,6 @@ static int local_open2(FsContext *fs_ctx, V9fsPath *dir_path, const char *name,
         buffer = rpath(fs_ctx, path);
         fd = open(buffer, flags, SM_LOCAL_MODE_BITS);
         if (fd == -1) {
-            g_free(buffer);
             err = fd;
             goto out;
         }
@@ -690,7 +683,6 @@ static int local_open2(FsContext *fs_ctx, V9fsPath *dir_path, const char *name,
         buffer = rpath(fs_ctx, path);
         fd = open(buffer, flags, SM_LOCAL_MODE_BITS);
         if (fd == -1) {
-            g_free(buffer);
             err = fd;
             goto out;
         }
@@ -706,7 +698,6 @@ static int local_open2(FsContext *fs_ctx, V9fsPath *dir_path, const char *name,
         buffer = rpath(fs_ctx, path);
         fd = open(buffer, flags, credp->fc_mode);
         if (fd == -1) {
-            g_free(buffer);
             err = fd;
             goto out;
         }
@@ -724,8 +715,8 @@ err_end:
     close(fd);
     remove(buffer);
     errno = serrno;
-    g_free(buffer);
 out:
+    g_free(buffer);
     v9fs_string_free(&fullname);
     return err;
 }
@@ -738,7 +729,7 @@ static int local_symlink(FsContext *fs_ctx, const char *oldpath,
     int serrno = 0;
     char *newpath;
     V9fsString fullname;
-    char *buffer;
+    char *buffer = NULL;
 
     v9fs_string_init(&fullname);
     v9fs_string_sprintf(&fullname, "%s/%s", dir_path->data, name);
@@ -751,7 +742,6 @@ static int local_symlink(FsContext *fs_ctx, const char *oldpath,
         buffer = rpath(fs_ctx, newpath);
         fd = open(buffer, O_CREAT|O_EXCL|O_RDWR|O_NOFOLLOW, SM_LOCAL_MODE_BITS);
         if (fd == -1) {
-            g_free(buffer);
             err = fd;
             goto out;
         }
@@ -781,7 +771,6 @@ static int local_symlink(FsContext *fs_ctx, const char *oldpath,
         buffer = rpath(fs_ctx, newpath);
         fd = open(buffer, O_CREAT|O_EXCL|O_RDWR|O_NOFOLLOW, SM_LOCAL_MODE_BITS);
         if (fd == -1) {
-            g_free(buffer);
             err = fd;
             goto out;
         }
@@ -810,7 +799,6 @@ static int local_symlink(FsContext *fs_ctx, const char *oldpath,
         buffer = rpath(fs_ctx, newpath);
         err = symlink(oldpath, buffer);
         if (err) {
-            g_free(buffer);
             goto out;
         }
         err = lchown(buffer, credp->fc_uid, credp->fc_gid);
@@ -831,8 +819,8 @@ static int local_symlink(FsContext *fs_ctx, const char *oldpath,
 err_end:
     remove(buffer);
     errno = serrno;
-    g_free(buffer);
 out:
+    g_free(buffer);
     v9fs_string_free(&fullname);
     return err;
 }
-- 
MST

[Qemu-devel] [PULL 3/8] pcie: correct mistaken register bit for End-End TLP Prefix Blocking

[Michael S. Tsirkin]

From: Chen Fan <chen.fan.fnst@cn.fujitsu.com>

from pcie spec 7.8.17, the End-End TLP Prefix Blocking bit local
is 15(e.g. 0x8000) in device control 2 register.

Signed-off-by: Chen Fan <chen.fan.fnst@cn.fujitsu.com>
Reviewed-by: Michael S. Tsirkin <mst@redhat.com>
Signed-off-by: Michael S. Tsirkin <mst@redhat.com>
---
 include/hw/pci/pcie_regs.h | 2 +-
 hw/pci/pcie.c              | 2 +-
 2 files changed, 2 insertions(+), 2 deletions(-)

diff --git a/include/hw/pci/pcie_regs.h b/include/hw/pci/pcie_regs.h
index 652d9fc..848ab1c 100644
--- a/include/hw/pci/pcie_regs.h
+++ b/include/hw/pci/pcie_regs.h
@@ -72,7 +72,7 @@
 #define PCI_EXP_DEVCAP2_EFF             0x100000
 #define PCI_EXP_DEVCAP2_EETLPP          0x200000
 
-#define PCI_EXP_DEVCTL2_EETLPPB         0x80
+#define PCI_EXP_DEVCTL2_EETLPPB         0x8000
 
 /* ARI */
 #define PCI_ARI_VER                     1
diff --git a/hw/pci/pcie.c b/hw/pci/pcie.c
index 1abbbb1..1463e65 100644
--- a/hw/pci/pcie.c
+++ b/hw/pci/pcie.c
@@ -84,7 +84,7 @@ int pcie_cap_init(PCIDevice *dev, uint8_t offset, uint8_t type, uint8_t port)
     pci_set_long(exp_cap + PCI_EXP_DEVCAP2,
                  PCI_EXP_DEVCAP2_EFF | PCI_EXP_DEVCAP2_EETLPP);
 
-    pci_set_word(dev->wmask + pos, PCI_EXP_DEVCTL2_EETLPPB);
+    pci_set_word(dev->wmask + pos + PCI_EXP_DEVCTL2, PCI_EXP_DEVCTL2_EETLPPB);
     return pos;
 }
 
-- 
MST

[Qemu-devel] [PULL 4/8] aer: fix wrong check on expose aer tlp prefix log

[Michael S. Tsirkin]

From: Chen Fan <chen.fan.fnst@cn.fujitsu.com>

when specify TLP Prefix log as using pcie_aer_inject_error,
the TLP prefix log is always discarded. because the check
is incorrect, the End-End TLP Prefix Supported bit
(PCI_EXP_DEVCAP2_EETLPP) should be in Device Capabilities 2 Register.

Signed-off-by: Chen Fan <chen.fan.fnst@cn.fujitsu.com>
Reviewed-by: Michael S. Tsirkin <mst@redhat.com>
Signed-off-by: Michael S. Tsirkin <mst@redhat.com>
---
 hw/pci/pcie_aer.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/hw/pci/pcie_aer.c b/hw/pci/pcie_aer.c
index 5a25c32..c7fad34 100644
--- a/hw/pci/pcie_aer.c
+++ b/hw/pci/pcie_aer.c
@@ -433,7 +433,7 @@ static void pcie_aer_update_log(PCIDevice *dev, const PCIEAERErr *err)
     }
 
     if ((err->flags & PCIE_AER_ERR_TLP_PREFIX_PRESENT) &&
-        (pci_get_long(dev->config + dev->exp.exp_cap + PCI_EXP_DEVCTL2) &
+        (pci_get_long(dev->config + dev->exp.exp_cap + PCI_EXP_DEVCAP2) &
          PCI_EXP_DEVCAP2_EETLPP)) {
         for (i = 0; i < ARRAY_SIZE(err->prefix); ++i) {
             /* 7.10.12 tlp prefix log register */
-- 
MST

[Qemu-devel] [PULL 5/8] pcie_aer: fix typos in pcie_aer_inject_error comment

[Michael S. Tsirkin]

From: Chen Fan <chen.fan.fnst@cn.fujitsu.com>

Refer to "PCI Express Base Spec3.0", this comments can't
fit the description in spec, so we should fix them.

Signed-off-by: Chen Fan <chen.fan.fnst@cn.fujitsu.com>
Reviewed-by: Michael S. Tsirkin <mst@redhat.com>
Signed-off-by: Michael S. Tsirkin <mst@redhat.com>
---
 hw/pci/pcie_aer.c | 6 +++---
 1 file changed, 3 insertions(+), 3 deletions(-)

diff --git a/hw/pci/pcie_aer.c b/hw/pci/pcie_aer.c
index c7fad34..9daebc2 100644
--- a/hw/pci/pcie_aer.c
+++ b/hw/pci/pcie_aer.c
@@ -618,11 +618,11 @@ static bool pcie_aer_inject_uncor_error(PCIEAERInject *inj, bool is_fatal)
  * non-Function specific error must be recorded in all functions.
  * It is the responsibility of the caller of this function.
  * It is also caller's responsibility to determine which function should
- * report the rerror.
+ * report the error.
  *
  * 6.2.4 Error Logging
- * 6.2.5 Sqeunce of Device Error Signaling and Logging Operations
- * table 6-2: Flowchard Showing Sequence of Device Error Signaling and Logging
+ * 6.2.5 Sequence of Device Error Signaling and Logging Operations
+ * table 6-2: Flowchart Showing Sequence of Device Error Signaling and Logging
  *            Operations
  */
 int pcie_aer_inject_error(PCIDevice *dev, const PCIEAERErr *err)
-- 
MST

[Qemu-devel] [PULL 6/8] aer: fix a wrong init PCI_ERR_COR_STATUS w1cmask type register

[Michael S. Tsirkin]

From: Chen Fan <chen.fan.fnst@cn.fujitsu.com>

Error Status Register, so this patch fix a wrong definition
for PCI_ERR_COR_STATUS register with w1cmask type.

Signed-off-by: Chen Fan <chen.fan.fnst@cn.fujitsu.com>
Reviewed-by: Michael S. Tsirkin <mst@redhat.com>
Signed-off-by: Michael S. Tsirkin <mst@redhat.com>
---
 hw/pci/pcie_aer.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/hw/pci/pcie_aer.c b/hw/pci/pcie_aer.c
index 9daebc2..9126058 100644
--- a/hw/pci/pcie_aer.c
+++ b/hw/pci/pcie_aer.c
@@ -123,7 +123,7 @@ int pcie_aer_init(PCIDevice *dev, uint16_t offset)
                  PCI_ERR_UNC_SUPPORTED);
 
     pci_long_test_and_set_mask(dev->w1cmask + offset + PCI_ERR_COR_STATUS,
-                               PCI_ERR_COR_STATUS);
+                               PCI_ERR_COR_SUPPORTED);
 
     pci_set_long(dev->config + offset + PCI_ERR_COR_MASK,
                  PCI_ERR_COR_MASK_DEFAULT);
-- 
MST

[Qemu-devel] [PULL 7/8] pci: fix several trivial typos in comment

[Michael S. Tsirkin]

From: Chen Fan <chen.fan.fnst@cn.fujitsu.com>

Signed-off-by: Chen Fan <chen.fan.fnst@cn.fujitsu.com>
Reviewed-by: Michael S. Tsirkin <mst@redhat.com>
Signed-off-by: Michael S. Tsirkin <mst@redhat.com>
---
 include/hw/pci/pci.h      | 2 +-
 include/hw/pci/pcie_aer.h | 2 +-
 2 files changed, 2 insertions(+), 2 deletions(-)

diff --git a/include/hw/pci/pci.h b/include/hw/pci/pci.h
index be2d9b8..b97c295 100644
--- a/include/hw/pci/pci.h
+++ b/include/hw/pci/pci.h
@@ -137,7 +137,7 @@ enum {
 #define PCI_CONFIG_HEADER_SIZE 0x40
 /* Size of the standard PCI config space */
 #define PCI_CONFIG_SPACE_SIZE 0x100
-/* Size of the standart PCIe config space: 4KB */
+/* Size of the standard PCIe config space: 4KB */
 #define PCIE_CONFIG_SPACE_SIZE  0x1000
 
 #define PCI_NUM_PINS 4 /* A-D */
diff --git a/include/hw/pci/pcie_aer.h b/include/hw/pci/pcie_aer.h
index bcac80a..2fb8388 100644
--- a/include/hw/pci/pcie_aer.h
+++ b/include/hw/pci/pcie_aer.h
@@ -51,7 +51,7 @@ struct PCIEAERLog {
     PCIEAERErr *log;
 };
 
-/* aer error message: error signaling message has only error sevirity and
+/* aer error message: error signaling message has only error severity and
    source id. See 2.2.8.3 error signaling messages */
 struct PCIEAERMsg {
     /*
-- 
MST

[Qemu-devel] [PULL 8/8] pcie_aer: fix comment to match pcie spec

[Michael S. Tsirkin]

Code comment says "table 6-2" but in fact it's is not a table, it is
"Figure 6-2" on page 479.

Cc: Chen Fan <chen.fan.fnst@cn.fujitsu.com>
Reported-by: Michael Tokarev <mjt@tls.msk.ru>
Signed-off-by: Michael S. Tsirkin <mst@redhat.com>
---
 hw/pci/pcie_aer.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/hw/pci/pcie_aer.c b/hw/pci/pcie_aer.c
index 9126058..eaa3e6e 100644
--- a/hw/pci/pcie_aer.c
+++ b/hw/pci/pcie_aer.c
@@ -622,8 +622,8 @@ static bool pcie_aer_inject_uncor_error(PCIEAERInject *inj, bool is_fatal)
  *
  * 6.2.4 Error Logging
  * 6.2.5 Sequence of Device Error Signaling and Logging Operations
- * table 6-2: Flowchart Showing Sequence of Device Error Signaling and Logging
- *            Operations
+ * Figure 6-2: Flowchart Showing Sequence of Device Error Signaling and Logging
+ *             Operations
  */
 int pcie_aer_inject_error(PCIDevice *dev, const PCIEAERErr *err)
 {
-- 
MST

Re: [Qemu-devel] [PULL 0/8] pci, virtio bugfixes for 2.3

[Peter Maydell]

On 18 March 2015 at 12:34, Michael S. Tsirkin <mst@redhat.com> wrote:
> The following changes since commit 18bf9e2f379334306530cbfd44218748eceaf67d:
>
>   virtio-scsi: remove empty wrapper for cmd (2015-03-11 18:24:30 +0100)
>
> are available in the git repository at:
>
>   git://git.kernel.org/pub/scm/virt/kvm/mst/qemu.git tags/for_upstream
>
> for you to fetch changes up to ce394947a75296fc10f1676932473e92aa8be11a:
>
>   pcie_aer: fix comment to match pcie spec (2015-03-18 12:48:21 +0100)
>
> ----------------------------------------------------------------
> pci, virtio bugfixes for 2.3
>
> Just a bunch of bugfixes. Should be nothing remarkable here.
>
> Signed-off-by: Michael S. Tsirkin <mst@redhat.com>
>
> ----------------------------------------------------------------

Applied, thanks.

-- PMM