[Qemu-devel] [PATCH] spice: set device address and device display ID in QXL interface

Lukáš Hrázký posted 1 patch 5 years, 1 month ago
Test docker-mingw@fedora passed
Test asan passed
Test checkpatch failed
Test docker-clang@ubuntu passed
Patches applied successfully (tree, apply log)
git fetch https://github.com/patchew-project/qemu tags/patchew/20190211141509.26964-1-lhrazky@redhat.com
Maintainers: Gerd Hoffmann <kraxel@redhat.com>
There is a newer version of this series
hw/display/qxl.c           | 14 ++++++++++-
include/ui/spice-display.h |  4 ++++
ui/spice-core.c            | 49 ++++++++++++++++++++++++++++++++++++++
ui/spice-display.c         | 11 +++++++++
4 files changed, 77 insertions(+), 1 deletion(-)
[Qemu-devel] [PATCH] spice: set device address and device display ID in QXL interface
Posted by Lukáš Hrázký 5 years, 1 month ago
Calls the new SPICE QXL interface function spice_qxl_set_device_info to
set the hardware address of the graphics device represented by the QXL
interface (e.g. a PCI path) and the device display IDs (the IDs of the
device's monitors that belong to this QXL interface).

Also stops using the deprecated spice_qxl_set_max_monitors, the new
interface function replaces it.

Signed-off-by: Lukáš Hrázký <lhrazky@redhat.com>
---
Hi,

I've fixed the undefined reference by checking the parent_dev pointer
directly instead of through pci_bus_is_root(), that was the only
function needed that's not built with PCI_CONFIG=n. Added a comment to
explain it too.

Changes since v2:
* Check the parent_dev pointer directly instead of calling
  pci_bus_is_root().

Cheers,
Lukas


 hw/display/qxl.c           | 14 ++++++++++-
 include/ui/spice-display.h |  4 ++++
 ui/spice-core.c            | 49 ++++++++++++++++++++++++++++++++++++++
 ui/spice-display.c         | 11 +++++++++
 4 files changed, 77 insertions(+), 1 deletion(-)

diff --git a/hw/display/qxl.c b/hw/display/qxl.c
index da8fd5a40a..c8ce5781e0 100644
--- a/hw/display/qxl.c
+++ b/hw/display/qxl.c
@@ -276,7 +276,8 @@ static void qxl_spice_monitors_config_async(PCIQXLDevice *qxl, int replay)
                     QXL_COOKIE_TYPE_POST_LOAD_MONITORS_CONFIG,
                     0));
     } else {
-#if SPICE_SERVER_VERSION >= 0x000c06 /* release 0.12.6 */
+/* >= release 0.12.6, < release 0.14.2 */
+#if SPICE_SERVER_VERSION >= 0x000c06 && SPICE_SERVER_VERSION < 0x000e02
         if (qxl->max_outputs) {
             spice_qxl_set_max_monitors(&qxl->ssd.qxl, qxl->max_outputs);
         }
@@ -2188,6 +2189,17 @@ static void qxl_realize_common(PCIQXLDevice *qxl, Error **errp)
                    SPICE_INTERFACE_QXL_MAJOR, SPICE_INTERFACE_QXL_MINOR);
         return;
     }
+
+#if SPICE_SERVER_VERSION >= 0x000e02 /* release 0.14.2 */
+    char device_address[256] = "";
+    if (qemu_spice_fill_device_address(qxl->vga.con, device_address, 256)) {
+        spice_qxl_set_device_info(&qxl->ssd.qxl,
+                                  device_address,
+                                  0,
+                                  qxl->max_outputs);
+    }
+#endif
+
     qemu_add_vm_change_state_handler(qxl_vm_change_state_handler, qxl);
 
     qxl->update_irq = qemu_bh_new(qxl_update_irq_bh, qxl);
diff --git a/include/ui/spice-display.h b/include/ui/spice-display.h
index 87a84a59d4..53c3612c32 100644
--- a/include/ui/spice-display.h
+++ b/include/ui/spice-display.h
@@ -179,3 +179,7 @@ void qemu_spice_wakeup(SimpleSpiceDisplay *ssd);
 void qemu_spice_display_start(void);
 void qemu_spice_display_stop(void);
 int qemu_spice_display_is_running(SimpleSpiceDisplay *ssd);
+
+bool qemu_spice_fill_device_address(QemuConsole *con,
+                                    char *device_address,
+                                    size_t size);
diff --git a/ui/spice-core.c b/ui/spice-core.c
index a40fb2c00d..a2f8e15030 100644
--- a/ui/spice-core.c
+++ b/ui/spice-core.c
@@ -34,6 +34,7 @@
 #include "qemu/option.h"
 #include "migration/misc.h"
 #include "hw/hw.h"
+#include "hw/pci/pci_bus.h"
 #include "ui/spice-display.h"
 
 /* core bits */
@@ -863,6 +864,54 @@ bool qemu_spice_have_display_interface(QemuConsole *con)
     return false;
 }
 
+/*
+ * Recursively (in reverse order) appends addresses of PCI devices as it moves
+ * up in the PCI hierarchy.
+ *
+ * @returns true on success, false when the buffer wasn't large enough
+ */
+static bool append_pci_address(char *buf, size_t buf_size, const PCIDevice *pci)
+{
+    PCIBus *bus = pci_get_bus(pci);
+    // equivalent to if (!pci_bus_is_root(bus)), but the function is not built
+    // with PCI_CONFIG=n, avoid using an #ifdef by checking directly
+    if (bus->parent_dev != NULL) {
+        append_pci_address(buf, buf_size, bus->parent_dev);
+    }
+
+    size_t len = strlen(buf);
+    ssize_t written = snprintf(buf + len, buf_size - len, "/%02x.%x",
+        PCI_SLOT(pci->devfn), PCI_FUNC(pci->devfn));
+
+    return written > 0 && written < buf_size - len;
+}
+
+bool qemu_spice_fill_device_address(QemuConsole *con,
+                                    char *device_address,
+                                    size_t size)
+{
+    DeviceState *dev = DEVICE(object_property_get_link(OBJECT(con),
+                                                       "device",
+                                                       &error_abort));
+    PCIDevice *pci = (PCIDevice *) object_dynamic_cast(OBJECT(dev),
+                                                       TYPE_PCI_DEVICE);
+
+    if (pci == NULL) {
+        warn_report("Setting device address of a display device to SPICE: "
+                    "Not a PCI device.");
+        return false;
+    }
+
+    strncpy(device_address, "pci/0000", size);
+    if (!append_pci_address(device_address, size, pci)) {
+        warn_report("Setting device address of a display device to SPICE: "
+            "Too many PCI devices in the chain.");
+        return false;
+    }
+
+    return true;
+}
+
 int qemu_spice_add_display_interface(QXLInstance *qxlin, QemuConsole *con)
 {
     if (g_slist_find(spice_consoles, con)) {
diff --git a/ui/spice-display.c b/ui/spice-display.c
index aea6f6ebce..a5e26479a8 100644
--- a/ui/spice-display.c
+++ b/ui/spice-display.c
@@ -1147,6 +1147,17 @@ static void qemu_spice_display_init_one(QemuConsole *con)
 
     ssd->qxl.base.sif = &dpy_interface.base;
     qemu_spice_add_display_interface(&ssd->qxl, con);
+
+#if SPICE_SERVER_VERSION >= 0x000e02 /* release 0.14.2 */
+    char device_address[256] = "";
+    if (qemu_spice_fill_device_address(con, device_address, 256)) {
+        spice_qxl_set_device_info(&ssd->qxl,
+                                  device_address,
+                                  qemu_console_get_head(con),
+                                  1);
+    }
+#endif
+
     qemu_spice_create_host_memslot(ssd);
 
     register_displaychangelistener(&ssd->dcl);
-- 
2.20.1


Re: [Qemu-devel] [PATCH] spice: set device address and device display ID in QXL interface
Posted by no-reply@patchew.org 5 years, 1 month ago
Patchew URL: https://patchew.org/QEMU/20190211141509.26964-1-lhrazky@redhat.com/



Hi,

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

Subject: [Qemu-devel] [PATCH] spice: set device address and device display ID in QXL interface
Type: series
Message-id: 20190211141509.26964-1-lhrazky@redhat.com

=== TEST SCRIPT BEGIN ===
#!/bin/bash
git config --local diff.renamelimit 0
git config --local diff.renames True
git config --local diff.algorithm histogram
./scripts/checkpatch.pl --mailback base..
=== TEST SCRIPT END ===

Updating 3c8cf5a9c21ff8782164d1def7f44bd888713384
From https://github.com/patchew-project/qemu
 * [new tag]               patchew/1550022537-27565-1-git-send-email-changpeng.liu@intel.com -> patchew/1550022537-27565-1-git-send-email-changpeng.liu@intel.com
 * [new tag]               patchew/1550064346-17012-1-git-send-email-wexu@redhat.com -> patchew/1550064346-17012-1-git-send-email-wexu@redhat.com
 * [new tag]               patchew/20190211141509.26964-1-lhrazky@redhat.com -> patchew/20190211141509.26964-1-lhrazky@redhat.com
 * [new tag]               patchew/20190213130240.15492-1-armbru@redhat.com -> patchew/20190213130240.15492-1-armbru@redhat.com
 * [new tag]               patchew/20190213145116.20654-1-yuri.benditovich@daynix.com -> patchew/20190213145116.20654-1-yuri.benditovich@daynix.com
Switched to a new branch 'test'
6043704883 spice: set device address and device display ID in QXL interface

=== OUTPUT BEGIN ===
ERROR: do not use C99 // comments
#87: FILE: ui/spice-core.c:876:
+    // equivalent to if (!pci_bus_is_root(bus)), but the function is not built

ERROR: do not use C99 // comments
#88: FILE: ui/spice-core.c:877:
+    // with PCI_CONFIG=n, avoid using an #ifdef by checking directly

total: 2 errors, 0 warnings, 111 lines checked

Commit 604370488342 (spice: set device address and device display ID in QXL interface) has style problems, please review.  If any of these errors
are false positives report them to the maintainer, see
CHECKPATCH in MAINTAINERS.
=== OUTPUT END ===

Test command exited with code: 1


The full log is available at
http://patchew.org/logs/20190211141509.26964-1-lhrazky@redhat.com/testing.checkpatch/?type=message.
---
Email generated automatically by Patchew [http://patchew.org/].
Please send your feedback to patchew-devel@redhat.com
[Qemu-devel] [PATCH v4] spice: set device address and device display ID in QXL interface
Posted by Lukáš Hrázký 5 years, 1 month ago
Calls the new SPICE QXL interface function spice_qxl_set_device_info to
set the hardware address of the graphics device represented by the QXL
interface (e.g. a PCI path) and the device display IDs (the IDs of the
device's monitors that belong to this QXL interface).

Also stops using the deprecated spice_qxl_set_max_monitors, the new
interface function replaces it.

Signed-off-by: Lukáš Hrázký <lhrazky@redhat.com>
---
Hi,

Changes since v3:
* Fix comments according to patchew check script.

Cheers,
Lukas


 hw/display/qxl.c           | 14 ++++++++++-
 include/ui/spice-display.h |  4 ++++
 ui/spice-core.c            | 49 ++++++++++++++++++++++++++++++++++++++
 ui/spice-display.c         | 11 +++++++++
 4 files changed, 77 insertions(+), 1 deletion(-)

diff --git a/hw/display/qxl.c b/hw/display/qxl.c
index da8fd5a40a..c8ce5781e0 100644
--- a/hw/display/qxl.c
+++ b/hw/display/qxl.c
@@ -276,7 +276,8 @@ static void qxl_spice_monitors_config_async(PCIQXLDevice *qxl, int replay)
                     QXL_COOKIE_TYPE_POST_LOAD_MONITORS_CONFIG,
                     0));
     } else {
-#if SPICE_SERVER_VERSION >= 0x000c06 /* release 0.12.6 */
+/* >= release 0.12.6, < release 0.14.2 */
+#if SPICE_SERVER_VERSION >= 0x000c06 && SPICE_SERVER_VERSION < 0x000e02
         if (qxl->max_outputs) {
             spice_qxl_set_max_monitors(&qxl->ssd.qxl, qxl->max_outputs);
         }
@@ -2188,6 +2189,17 @@ static void qxl_realize_common(PCIQXLDevice *qxl, Error **errp)
                    SPICE_INTERFACE_QXL_MAJOR, SPICE_INTERFACE_QXL_MINOR);
         return;
     }
+
+#if SPICE_SERVER_VERSION >= 0x000e02 /* release 0.14.2 */
+    char device_address[256] = "";
+    if (qemu_spice_fill_device_address(qxl->vga.con, device_address, 256)) {
+        spice_qxl_set_device_info(&qxl->ssd.qxl,
+                                  device_address,
+                                  0,
+                                  qxl->max_outputs);
+    }
+#endif
+
     qemu_add_vm_change_state_handler(qxl_vm_change_state_handler, qxl);
 
     qxl->update_irq = qemu_bh_new(qxl_update_irq_bh, qxl);
diff --git a/include/ui/spice-display.h b/include/ui/spice-display.h
index 87a84a59d4..53c3612c32 100644
--- a/include/ui/spice-display.h
+++ b/include/ui/spice-display.h
@@ -179,3 +179,7 @@ void qemu_spice_wakeup(SimpleSpiceDisplay *ssd);
 void qemu_spice_display_start(void);
 void qemu_spice_display_stop(void);
 int qemu_spice_display_is_running(SimpleSpiceDisplay *ssd);
+
+bool qemu_spice_fill_device_address(QemuConsole *con,
+                                    char *device_address,
+                                    size_t size);
diff --git a/ui/spice-core.c b/ui/spice-core.c
index a40fb2c00d..4b501d485e 100644
--- a/ui/spice-core.c
+++ b/ui/spice-core.c
@@ -34,6 +34,7 @@
 #include "qemu/option.h"
 #include "migration/misc.h"
 #include "hw/hw.h"
+#include "hw/pci/pci_bus.h"
 #include "ui/spice-display.h"
 
 /* core bits */
@@ -863,6 +864,54 @@ bool qemu_spice_have_display_interface(QemuConsole *con)
     return false;
 }
 
+/*
+ * Recursively (in reverse order) appends addresses of PCI devices as it moves
+ * up in the PCI hierarchy.
+ *
+ * @returns true on success, false when the buffer wasn't large enough
+ */
+static bool append_pci_address(char *buf, size_t buf_size, const PCIDevice *pci)
+{
+    PCIBus *bus = pci_get_bus(pci);
+    /* equivalent to if (!pci_bus_is_root(bus)), but the function is not built
+       with PCI_CONFIG=n, avoid using an #ifdef by checking directly */
+    if (bus->parent_dev != NULL) {
+        append_pci_address(buf, buf_size, bus->parent_dev);
+    }
+
+    size_t len = strlen(buf);
+    ssize_t written = snprintf(buf + len, buf_size - len, "/%02x.%x",
+        PCI_SLOT(pci->devfn), PCI_FUNC(pci->devfn));
+
+    return written > 0 && written < buf_size - len;
+}
+
+bool qemu_spice_fill_device_address(QemuConsole *con,
+                                    char *device_address,
+                                    size_t size)
+{
+    DeviceState *dev = DEVICE(object_property_get_link(OBJECT(con),
+                                                       "device",
+                                                       &error_abort));
+    PCIDevice *pci = (PCIDevice *) object_dynamic_cast(OBJECT(dev),
+                                                       TYPE_PCI_DEVICE);
+
+    if (pci == NULL) {
+        warn_report("Setting device address of a display device to SPICE: "
+                    "Not a PCI device.");
+        return false;
+    }
+
+    strncpy(device_address, "pci/0000", size);
+    if (!append_pci_address(device_address, size, pci)) {
+        warn_report("Setting device address of a display device to SPICE: "
+            "Too many PCI devices in the chain.");
+        return false;
+    }
+
+    return true;
+}
+
 int qemu_spice_add_display_interface(QXLInstance *qxlin, QemuConsole *con)
 {
     if (g_slist_find(spice_consoles, con)) {
diff --git a/ui/spice-display.c b/ui/spice-display.c
index aea6f6ebce..a5e26479a8 100644
--- a/ui/spice-display.c
+++ b/ui/spice-display.c
@@ -1147,6 +1147,17 @@ static void qemu_spice_display_init_one(QemuConsole *con)
 
     ssd->qxl.base.sif = &dpy_interface.base;
     qemu_spice_add_display_interface(&ssd->qxl, con);
+
+#if SPICE_SERVER_VERSION >= 0x000e02 /* release 0.14.2 */
+    char device_address[256] = "";
+    if (qemu_spice_fill_device_address(con, device_address, 256)) {
+        spice_qxl_set_device_info(&ssd->qxl,
+                                  device_address,
+                                  qemu_console_get_head(con),
+                                  1);
+    }
+#endif
+
     qemu_spice_create_host_memslot(ssd);
 
     register_displaychangelistener(&ssd->dcl);
-- 
2.20.1


[Qemu-devel] [PATCH v5] spice: set device address and device display ID in QXL interface
Posted by Lukáš Hrázký 5 years, 1 month ago
Calls the new SPICE QXL interface function spice_qxl_set_device_info to
set the hardware address of the graphics device represented by the QXL
interface (e.g. a PCI path) and the device display IDs (the IDs of the
device's monitors that belong to this QXL interface).

Also stops using the deprecated spice_qxl_set_max_monitors, the new
interface function replaces it.

Signed-off-by: Lukáš Hrázký <lhrazky@redhat.com>
---
Hi,

sorry about the noise, v4 still isn't passing the check. I didn't know
the check script is in the git tree. This one should pass.

Changes since v4:
* Really fix comments according to the check script.

Cheers,
Lukas


 hw/display/qxl.c           | 14 ++++++++++-
 include/ui/spice-display.h |  4 +++
 ui/spice-core.c            | 51 ++++++++++++++++++++++++++++++++++++++
 ui/spice-display.c         | 11 ++++++++
 4 files changed, 79 insertions(+), 1 deletion(-)

diff --git a/hw/display/qxl.c b/hw/display/qxl.c
index da8fd5a40a..c8ce5781e0 100644
--- a/hw/display/qxl.c
+++ b/hw/display/qxl.c
@@ -276,7 +276,8 @@ static void qxl_spice_monitors_config_async(PCIQXLDevice *qxl, int replay)
                     QXL_COOKIE_TYPE_POST_LOAD_MONITORS_CONFIG,
                     0));
     } else {
-#if SPICE_SERVER_VERSION >= 0x000c06 /* release 0.12.6 */
+/* >= release 0.12.6, < release 0.14.2 */
+#if SPICE_SERVER_VERSION >= 0x000c06 && SPICE_SERVER_VERSION < 0x000e02
         if (qxl->max_outputs) {
             spice_qxl_set_max_monitors(&qxl->ssd.qxl, qxl->max_outputs);
         }
@@ -2188,6 +2189,17 @@ static void qxl_realize_common(PCIQXLDevice *qxl, Error **errp)
                    SPICE_INTERFACE_QXL_MAJOR, SPICE_INTERFACE_QXL_MINOR);
         return;
     }
+
+#if SPICE_SERVER_VERSION >= 0x000e02 /* release 0.14.2 */
+    char device_address[256] = "";
+    if (qemu_spice_fill_device_address(qxl->vga.con, device_address, 256)) {
+        spice_qxl_set_device_info(&qxl->ssd.qxl,
+                                  device_address,
+                                  0,
+                                  qxl->max_outputs);
+    }
+#endif
+
     qemu_add_vm_change_state_handler(qxl_vm_change_state_handler, qxl);
 
     qxl->update_irq = qemu_bh_new(qxl_update_irq_bh, qxl);
diff --git a/include/ui/spice-display.h b/include/ui/spice-display.h
index 87a84a59d4..53c3612c32 100644
--- a/include/ui/spice-display.h
+++ b/include/ui/spice-display.h
@@ -179,3 +179,7 @@ void qemu_spice_wakeup(SimpleSpiceDisplay *ssd);
 void qemu_spice_display_start(void);
 void qemu_spice_display_stop(void);
 int qemu_spice_display_is_running(SimpleSpiceDisplay *ssd);
+
+bool qemu_spice_fill_device_address(QemuConsole *con,
+                                    char *device_address,
+                                    size_t size);
diff --git a/ui/spice-core.c b/ui/spice-core.c
index a40fb2c00d..37fae3c424 100644
--- a/ui/spice-core.c
+++ b/ui/spice-core.c
@@ -34,6 +34,7 @@
 #include "qemu/option.h"
 #include "migration/misc.h"
 #include "hw/hw.h"
+#include "hw/pci/pci_bus.h"
 #include "ui/spice-display.h"
 
 /* core bits */
@@ -863,6 +864,56 @@ bool qemu_spice_have_display_interface(QemuConsole *con)
     return false;
 }
 
+/*
+ * Recursively (in reverse order) appends addresses of PCI devices as it moves
+ * up in the PCI hierarchy.
+ *
+ * @returns true on success, false when the buffer wasn't large enough
+ */
+static bool append_pci_address(char *buf, size_t buf_size, const PCIDevice *pci)
+{
+    PCIBus *bus = pci_get_bus(pci);
+    /*
+     * equivalent to if (!pci_bus_is_root(bus)), but the function is not built
+     * with PCI_CONFIG=n, avoid using an #ifdef by checking directly
+     */
+    if (bus->parent_dev != NULL) {
+        append_pci_address(buf, buf_size, bus->parent_dev);
+    }
+
+    size_t len = strlen(buf);
+    ssize_t written = snprintf(buf + len, buf_size - len, "/%02x.%x",
+        PCI_SLOT(pci->devfn), PCI_FUNC(pci->devfn));
+
+    return written > 0 && written < buf_size - len;
+}
+
+bool qemu_spice_fill_device_address(QemuConsole *con,
+                                    char *device_address,
+                                    size_t size)
+{
+    DeviceState *dev = DEVICE(object_property_get_link(OBJECT(con),
+                                                       "device",
+                                                       &error_abort));
+    PCIDevice *pci = (PCIDevice *) object_dynamic_cast(OBJECT(dev),
+                                                       TYPE_PCI_DEVICE);
+
+    if (pci == NULL) {
+        warn_report("Setting device address of a display device to SPICE: "
+                    "Not a PCI device.");
+        return false;
+    }
+
+    strncpy(device_address, "pci/0000", size);
+    if (!append_pci_address(device_address, size, pci)) {
+        warn_report("Setting device address of a display device to SPICE: "
+            "Too many PCI devices in the chain.");
+        return false;
+    }
+
+    return true;
+}
+
 int qemu_spice_add_display_interface(QXLInstance *qxlin, QemuConsole *con)
 {
     if (g_slist_find(spice_consoles, con)) {
diff --git a/ui/spice-display.c b/ui/spice-display.c
index aea6f6ebce..a5e26479a8 100644
--- a/ui/spice-display.c
+++ b/ui/spice-display.c
@@ -1147,6 +1147,17 @@ static void qemu_spice_display_init_one(QemuConsole *con)
 
     ssd->qxl.base.sif = &dpy_interface.base;
     qemu_spice_add_display_interface(&ssd->qxl, con);
+
+#if SPICE_SERVER_VERSION >= 0x000e02 /* release 0.14.2 */
+    char device_address[256] = "";
+    if (qemu_spice_fill_device_address(con, device_address, 256)) {
+        spice_qxl_set_device_info(&ssd->qxl,
+                                  device_address,
+                                  qemu_console_get_head(con),
+                                  1);
+    }
+#endif
+
     qemu_spice_create_host_memslot(ssd);
 
     register_displaychangelistener(&ssd->dcl);
-- 
2.20.1


Re: [Qemu-devel] [PATCH v5] spice: set device address and device display ID in QXL interface
Posted by Gerd Hoffmann 5 years, 1 month ago
On Fri, Feb 15, 2019 at 04:09:19PM +0100, Lukáš Hrázký wrote:
> Calls the new SPICE QXL interface function spice_qxl_set_device_info to
> set the hardware address of the graphics device represented by the QXL
> interface (e.g. a PCI path) and the device display IDs (the IDs of the
> device's monitors that belong to this QXL interface).
> 
> Also stops using the deprecated spice_qxl_set_max_monitors, the new
> interface function replaces it.

Added to UI queue.

thanks,
  Gerd