Re: [Qemu-devel] [PATCH v2 2/3] vfio/display: add xres + yres properties
On Thu, 21 Feb 2019 08:46:43 +0100
Gerd Hoffmann wrote:
> Hi,
>
> > > +DEFINE_PROP_UINT32("xres", VFIOPCIDevice, display_xres, 0),
> > > +DEFINE_PROP_UINT32("yres", VFIOPCIDevice, display_yres, 0),
> >
> > This is actually quite fun, I started my VM with arbitrary numbers and
> > the Windows GUI honored it every time. Probably very useful for
> > playing with odd screen sizes. I also tried to break it using
> > 100x100, but the display came up as 1920x1200, the maximum
> > resolution GVT-g supports for this type. I don't see that QEMU is
> > bounding this though, do we depend on the mdev device to ignore it if
> > we pass values it cannot support?
>
> There is a check in vfio_display_edid_update().
Ok, so we're bounded within QEMU, that seems good. Thanks,
Alex
Re: [Qemu-devel] [PATCH v2 2/3] vfio/display: add xres + yres properties
Hi,
> > +DEFINE_PROP_UINT32("xres", VFIOPCIDevice, display_xres, 0),
> > +DEFINE_PROP_UINT32("yres", VFIOPCIDevice, display_yres, 0),
>
> This is actually quite fun, I started my VM with arbitrary numbers and
> the Windows GUI honored it every time. Probably very useful for
> playing with odd screen sizes. I also tried to break it using
> 100x100, but the display came up as 1920x1200, the maximum
> resolution GVT-g supports for this type. I don't see that QEMU is
> bounding this though, do we depend on the mdev device to ignore it if
> we pass values it cannot support?
There is a check in vfio_display_edid_update().
cheers,
Gerd
Re: [Qemu-devel] [PATCH v2 2/3] vfio/display: add xres + yres properties
On Wed, 20 Feb 2019 09:47:52 +0100
Gerd Hoffmann wrote:
> This allows configure the display resolution which the vgpu should use.
> The information will be passed to the guest using EDID, so the mdev
> driver must support the vfio edid region for this to work.
>
> Signed-off-by: Gerd Hoffmann
> ---
> hw/vfio/pci.h | 2 ++
> hw/vfio/display.c | 16 ++--
> hw/vfio/pci.c | 2 ++
> 3 files changed, 18 insertions(+), 2 deletions(-)
>
> diff --git a/hw/vfio/pci.h b/hw/vfio/pci.h
> index b1ae4c0754..c11c3f1670 100644
> --- a/hw/vfio/pci.h
> +++ b/hw/vfio/pci.h
> @@ -149,6 +149,8 @@ typedef struct VFIOPCIDevice {
> #define VFIO_FEATURE_ENABLE_IGD_OPREGION \
> (1 << VFIO_FEATURE_ENABLE_IGD_OPREGION_BIT)
> OnOffAuto display;
> +uint32_t display_xres;
> +uint32_t display_yres;
> int32_t bootindex;
> uint32_t igd_gms;
> OffAutoPCIBAR msix_relo;
> diff --git a/hw/vfio/display.c b/hw/vfio/display.c
> index ed2eb19ea3..7b9b604a64 100644
> --- a/hw/vfio/display.c
> +++ b/hw/vfio/display.c
> @@ -46,8 +46,8 @@ static void vfio_display_edid_update(VFIOPCIDevice *vdev,
> bool enabled,
> qemu_edid_info edid = {
> .maxx = dpy->edid_regs->max_xres,
> .maxy = dpy->edid_regs->max_yres,
> -.prefx = prefx,
> -.prefy = prefy,
> +.prefx = prefx ?: vdev->display_xres,
> +.prefy = prefy ?: vdev->display_yres,
> };
>
> dpy->edid_regs->link_state = VFIO_DEVICE_GFX_LINK_STATE_DOWN;
> @@ -117,6 +117,10 @@ static void vfio_display_edid_init(VFIOPCIDevice *vdev)
> VFIO_REGION_SUBTYPE_GFX_EDID,
> >edid_info);
> if (ret) {
> +if (vdev->display_xres || vdev->display_yres) {
> +warn_report("vfio: no edid support available, "
> +"xres and yres properties have no effect.");
> +}
In order to get here the device needs to have a display option set to
'on' or 'auto' and that display needs to be backed by a dmabuf graphics
plane. That means that QEMU is happy to run without any warning if a
user sets a resolution on a region backed display, or a device without
a display. I think that QEMU should probably fail, not just warn, for
all cases where an option is not appropriate for a device. Perhaps
EDID setup should set a feature bit or flag that we can test similar to
how and where we test for a stray ramfb option.
> return;
> }
>
> @@ -128,6 +132,14 @@ static void vfio_display_edid_init(VFIOPCIDevice *vdev)
> pread_field(fd, dpy->edid_info, dpy->edid_regs, max_yres);
> dpy->edid_blob = g_malloc0(dpy->edid_regs->edid_max_size);
>
> +/* if xres + yres properties are unset use the maximum resolution */
> +if (!vdev->display_xres) {
> +vdev->display_xres = dpy->edid_regs->max_xres;
> +}
> +if (!vdev->display_yres) {
> +vdev->display_yres = dpy->edid_regs->max_yres;
> +}
> +
> vfio_display_edid_update(vdev, true, 0, 0);
> return;
>
> diff --git a/hw/vfio/pci.c b/hw/vfio/pci.c
> index dd12f36391..edb8394038 100644
> --- a/hw/vfio/pci.c
> +++ b/hw/vfio/pci.c
> @@ -3182,6 +3182,8 @@ static Property vfio_pci_dev_properties[] = {
> DEFINE_PROP_STRING("sysfsdev", VFIOPCIDevice, vbasedev.sysfsdev),
> DEFINE_PROP_ON_OFF_AUTO("display", VFIOPCIDevice,
> display, ON_OFF_AUTO_OFF),
> +DEFINE_PROP_UINT32("xres", VFIOPCIDevice, display_xres, 0),
> +DEFINE_PROP_UINT32("yres", VFIOPCIDevice, display_yres, 0),
This is actually quite fun, I started my VM with arbitrary numbers and
the Windows GUI honored it every time. Probably very useful for
playing with odd screen sizes. I also tried to break it using
100x100, but the display came up as 1920x1200, the maximum
resolution GVT-g supports for this type. I don't see that QEMU is
bounding this though, do we depend on the mdev device to ignore it if
we pass values it cannot support? Thanks,
Alex
> DEFINE_PROP_UINT32("x-intx-mmap-timeout-ms", VFIOPCIDevice,
> intx.mmap_timeout, 1100),
> DEFINE_PROP_BIT("x-vga", VFIOPCIDevice, features,
[Qemu-devel] [PATCH v2 2/3] vfio/display: add xres + yres properties
This allows configure the display resolution which the vgpu should use.
The information will be passed to the guest using EDID, so the mdev
driver must support the vfio edid region for this to work.
Signed-off-by: Gerd Hoffmann
---
hw/vfio/pci.h | 2 ++
hw/vfio/display.c | 16 ++--
hw/vfio/pci.c | 2 ++
3 files changed, 18 insertions(+), 2 deletions(-)
diff --git a/hw/vfio/pci.h b/hw/vfio/pci.h
index b1ae4c0754..c11c3f1670 100644
--- a/hw/vfio/pci.h
+++ b/hw/vfio/pci.h
@@ -149,6 +149,8 @@ typedef struct VFIOPCIDevice {
#define VFIO_FEATURE_ENABLE_IGD_OPREGION \
(1 << VFIO_FEATURE_ENABLE_IGD_OPREGION_BIT)
OnOffAuto display;
+uint32_t display_xres;
+uint32_t display_yres;
int32_t bootindex;
uint32_t igd_gms;
OffAutoPCIBAR msix_relo;
diff --git a/hw/vfio/display.c b/hw/vfio/display.c
index ed2eb19ea3..7b9b604a64 100644
--- a/hw/vfio/display.c
+++ b/hw/vfio/display.c
@@ -46,8 +46,8 @@ static void vfio_display_edid_update(VFIOPCIDevice *vdev,
bool enabled,
qemu_edid_info edid = {
.maxx = dpy->edid_regs->max_xres,
.maxy = dpy->edid_regs->max_yres,
-.prefx = prefx,
-.prefy = prefy,
+.prefx = prefx ?: vdev->display_xres,
+.prefy = prefy ?: vdev->display_yres,
};
dpy->edid_regs->link_state = VFIO_DEVICE_GFX_LINK_STATE_DOWN;
@@ -117,6 +117,10 @@ static void vfio_display_edid_init(VFIOPCIDevice *vdev)
VFIO_REGION_SUBTYPE_GFX_EDID,
>edid_info);
if (ret) {
+if (vdev->display_xres || vdev->display_yres) {
+warn_report("vfio: no edid support available, "
+"xres and yres properties have no effect.");
+}
return;
}
@@ -128,6 +132,14 @@ static void vfio_display_edid_init(VFIOPCIDevice *vdev)
pread_field(fd, dpy->edid_info, dpy->edid_regs, max_yres);
dpy->edid_blob = g_malloc0(dpy->edid_regs->edid_max_size);
+/* if xres + yres properties are unset use the maximum resolution */
+if (!vdev->display_xres) {
+vdev->display_xres = dpy->edid_regs->max_xres;
+}
+if (!vdev->display_yres) {
+vdev->display_yres = dpy->edid_regs->max_yres;
+}
+
vfio_display_edid_update(vdev, true, 0, 0);
return;
diff --git a/hw/vfio/pci.c b/hw/vfio/pci.c
index dd12f36391..edb8394038 100644
--- a/hw/vfio/pci.c
+++ b/hw/vfio/pci.c
@@ -3182,6 +3182,8 @@ static Property vfio_pci_dev_properties[] = {
DEFINE_PROP_STRING("sysfsdev", VFIOPCIDevice, vbasedev.sysfsdev),
DEFINE_PROP_ON_OFF_AUTO("display", VFIOPCIDevice,
display, ON_OFF_AUTO_OFF),
+DEFINE_PROP_UINT32("xres", VFIOPCIDevice, display_xres, 0),
+DEFINE_PROP_UINT32("yres", VFIOPCIDevice, display_yres, 0),
DEFINE_PROP_UINT32("x-intx-mmap-timeout-ms", VFIOPCIDevice,
intx.mmap_timeout, 1100),
DEFINE_PROP_BIT("x-vga", VFIOPCIDevice, features,
--
2.9.3
