Re: [PULL 1/1] qxl: introduce hardware revision 5

[Yuri Benditovich <ybendito@redhat.com>]

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

----- Original Message -----

> From: "Laszlo Ersek" <lersek@redhat.com>
> To: "Gerd Hoffmann" <kraxel@redhat.com>
> Cc: qemu-devel@nongnu.org, "Eduardo Habkost" <ehabkost@redhat.com>, "Yuri
> Benditovich" <ybendito@redhat.com>, "Yan Vugenfirer" <yvugenfi@redhat.com>
> Sent: Thursday, March 5, 2020 1:28:23 AM
> Subject: Re: [PULL 1/1] qxl: introduce hardware revision 5

> Hi Gerd,

> On 02/13/20 10:06, Gerd Hoffmann wrote:
> > The only difference to hardware revision 4 is that the device doesn't
> > switch to VGA mode in case someone happens to touch a VGA register,
> > which should make things more robust in configurations with multiple
> > vga devices.
> >
> > Swtiching back to VGA mode happens on reset, either full machine
> > reset or qxl device reset (QXL_IO_RESET ioport command).
> >
> > Signed-off-by: Gerd Hoffmann <kraxel@redhat.com>
> > Reviewed-by: Maxim Levitsky <mlevitsk@redhat.com>
> > Message-id: 20200206074358.4274-1-kraxel@redhat.com
> > ---
> > hw/display/qxl.h | 2 +-
> > hw/core/machine.c | 2 ++
> > hw/display/qxl.c | 7 ++++++-
> > 3 files changed, 9 insertions(+), 2 deletions(-)
> >
> > diff --git a/hw/display/qxl.h b/hw/display/qxl.h
> > index 80eb0d267269..707631a1f573 100644
> > --- a/hw/display/qxl.h
> > +++ b/hw/display/qxl.h
> > @@ -144,7 +144,7 @@ typedef struct PCIQXLDevice {
> > } \
> > } while (0)
> >
> > -#define QXL_DEFAULT_REVISION QXL_REVISION_STABLE_V12
> > +#define QXL_DEFAULT_REVISION (QXL_REVISION_STABLE_V12 + 1)
> >
> > /* qxl.c */
> > void *qxl_phys2virt(PCIQXLDevice *qxl, QXLPHYSICAL phys, int group_id);
> > diff --git a/hw/core/machine.c b/hw/core/machine.c
> > index d8e30e4895d8..84812a1d1cc1 100644
> > --- a/hw/core/machine.c
> > +++ b/hw/core/machine.c
> > @@ -34,6 +34,8 @@ GlobalProperty hw_compat_4_2[] = {
> > { "vhost-blk-device", "seg_max_adjust", "off"},
> > { "usb-host", "suppress-remote-wake", "off" },
> > { "usb-redir", "suppress-remote-wake", "off" },
> > + { "qxl", "revision", "4" },
> > + { "qxl-vga", "revision", "4" },
> > };
> > const size_t hw_compat_4_2_len = G_N_ELEMENTS(hw_compat_4_2);
> >
> > diff --git a/hw/display/qxl.c b/hw/display/qxl.c
> > index c33b1915a52c..64884da70857 100644
> > --- a/hw/display/qxl.c
> > +++ b/hw/display/qxl.c
> > @@ -1309,7 +1309,8 @@ static void qxl_vga_ioport_write(void *opaque,
> > uint32_t addr, uint32_t val)
> > PCIQXLDevice *qxl = container_of(vga, PCIQXLDevice, vga);
> >
> > trace_qxl_io_write_vga(qxl->id, qxl_mode_to_string(qxl->mode), addr, val);
> > - if (qxl->mode != QXL_MODE_VGA) {
> > + if (qxl->mode != QXL_MODE_VGA &&
> > + qxl->revision <= QXL_REVISION_STABLE_V12) {
> > qxl_destroy_primary(qxl, QXL_SYNC);
> > qxl_soft_reset(qxl);
> > }
> > @@ -2121,6 +2122,10 @@ static void qxl_realize_common(PCIQXLDevice *qxl,
> > Error **errp)
> > pci_device_rev = QXL_REVISION_STABLE_V12;
> > io_size = pow2ceil(QXL_IO_RANGE_SIZE);
> > break;
> > + case 5: /* qxl-5 */
> > + pci_device_rev = QXL_REVISION_STABLE_V12 + 1;
> > + io_size = pow2ceil(QXL_IO_RANGE_SIZE);
> > + break;
> > default:
> > error_setg(errp, "Invalid revision %d for qxl device (max %d)",
> > qxl->revision, QXL_DEFAULT_REVISION);
> >

> this patch -- commit ed71c09ffd6f -- disables ACPI S3 in the Windows 10
> guest for me, using OVMF and QXL.

> The "Sleep" menu entry disappears from the power button icon/menu at the
> login screen, and "psshutdown -d -t 3" (from the pstools package) also
> stops working (it reports that the computer does not support S3).

> At the parent commit (e18e5501d8ac), S3 suspend/resume works.

> Here's the bisection log:

> > git bisect start
> > # good: [928173659d6e5dc368284f73f90ea1d129e1f57d] Merge remote-tracking
> > branch 'remotes/pmaydell/tags/pull-target-arm-20200130' into staging
> > git bisect good 928173659d6e5dc368284f73f90ea1d129e1f57d
> > # good: [93c86fff53a267f657e79ec07dcd04b63882e330] Merge remote-tracking
> > branch 'remotes/pmaydell/tags/pull-target-arm-20200207' into staging
> > git bisect good 93c86fff53a267f657e79ec07dcd04b63882e330
> > # bad: [db736e0437aa6fd7c1b7e4599c17f9619ab6b837] Merge remote-tracking
> > branch 'remotes/bonzini/tags/for-upstream' into staging
> > git bisect bad db736e0437aa6fd7c1b7e4599c17f9619ab6b837
> > # bad: [8ee06e4ccb0f447caf9dc884b98986c155915e5c] ppc/mac_oldworld: use
> > memdev for RAM
> > git bisect bad 8ee06e4ccb0f447caf9dc884b98986c155915e5c
> > # good: [dc7a88d0810ad272bdcd2e0869359af78fdd9114] target/arm: Implement
> > ARMv8.1-VMID16 extension
> > git bisect good dc7a88d0810ad272bdcd2e0869359af78fdd9114
> > # bad: [652c5bbd7e7d3cb3d27a2e0590118dc79fb6f4d8] Merge remote-tracking
> > branch 'remotes/vivier2/tags/linux-user-for-5.0-pull-request' into staging
> > git bisect bad 652c5bbd7e7d3cb3d27a2e0590118dc79fb6f4d8
> > # bad: [e050e426782ec4ae6bf84e5ec75ca502187f69cb] qapi: Use explicit
> > bulleted lists
> > git bisect bad e050e426782ec4ae6bf84e5ec75ca502187f69cb
> > # good: [5d6542bea780ad443c4f7f1496e64706101f525c] Merge remote-tracking
> > branch 'remotes/rth/tags/pull-tcg-20200212' into staging
> > git bisect good 5d6542bea780ad443c4f7f1496e64706101f525c
> > # bad: [72ec8bf362b24ebbd45452c298a3b14fb617eebb] qga/qapi-schema.json: Fix
> > missing '-' in GuestDiskBusType doc comment
> > git bisect bad 72ec8bf362b24ebbd45452c298a3b14fb617eebb
> > # bad: [2eb054c237d0b5427f62499f3c31e90cf87821d7] configure: Allow user to
> > specify sphinx-build binary
> > git bisect bad 2eb054c237d0b5427f62499f3c31e90cf87821d7
> > # bad: [517c84cef759a453cfb8f51498aebc909a5f3b39] Merge remote-tracking
> > branch 'remotes/kraxel/tags/vga-20200213-pull-request' into staging
> > git bisect bad 517c84cef759a453cfb8f51498aebc909a5f3b39
> > # bad: [ed71c09ffd6fbd01c2a487d47291ae57b08671ea] qxl: introduce hardware
> > revision 5
> > git bisect bad ed71c09ffd6fbd01c2a487d47291ae57b08671ea
> > # first bad commit: [ed71c09ffd6fbd01c2a487d47291ae57b08671ea] qxl:
> > introduce hardware revision 5

> OVMF is built at current edk2 master: e63d54db9526.

> In the Windows 10 guest, the installed driver has the following
> properties (per Device Manager):

> - driver date: 7/28/2015
> - driver version: 22.33.46.473
This is not what I would expect from any qxl driver for Win10 
What is the name of display adapter? 
The driver provider is 'Red Hat'? 
Does your guest run with secure boot? 
I do not see such an effect with 'Sleep' with latest qemu master + OVMF + qxl + Win10, the sleep button does not disappear. 

> I tried upgrading the QXL driver in the Windows 10 guest, using the
> latest package at
> <https://fedorapeople.org/groups/virt/virtio-win/repo/latest/?C=M;O=D>,
> namely "virtio-win-0.1.173-7.noarch.rpm". But Device Manager said that
> the currently installed driver was already the best / most recent for
> the device.For now I'll force the revision back to 4 using <qemu:arg>
> elements in
> my domain XML. (I'd like to use pc-q35-5.0.)

> <qemu:arg value='-global'/>
> <qemu:arg value='qxl-vga.revision=4'/>

> Thanks
> Laszlo

[-- Attachment #2: Type: text/html, Size: 9262 bytes --]