[Spice-devel] [qxl-wddm-dod] Fix for black screen on driver uninstall on ovmf platform

[Frediano Ziglio]

> 
> https://bugzilla.redhat.com/show_bug.cgi?id=1661147
> If display miniport driver does not pass valid display info to
> the basic display driver on disable/uninstall, the basic
> display driver raises run-time error and stays with yellow
> bang. This behavior is specific to UEFI machines.
> Reference:
> https://docs.microsoft.com/en-us/windows-hardware/drivers/ddi/content/dispmprt/nc-dispmprt-dxgkddi_stop_device_and_release_post_display_ownership
> 
> Signed-off-by: Yuri Benditovich <yuri.benditovich at daynix.com>
> ---
>  qxldod/QxlDod.cpp | 47 ++++++++++++++++++++++++++++++++++++++++++++++-
>  qxldod/QxlDod.h   |  6 +++++-
>  2 files changed, 51 insertions(+), 2 deletions(-)
> 
> diff --git a/qxldod/QxlDod.cpp b/qxldod/QxlDod.cpp
> index b72e12c..dea78e2 100755
> --- a/qxldod/QxlDod.cpp
> +++ b/qxldod/QxlDod.cpp
> @@ -189,6 +189,12 @@ NTSTATUS QxlDod::StartDevice(_In_  DXGK_START_INFO*
> pDxgkStartInfo,
>          return Status;
>      }
>  
> +    DXGK_DISPLAY_INFORMATION *pInfo = &m_CurrentModes[0].DispInfo;
> +    DbgPrint(TRACE_LEVEL_INFORMATION, ("%s: Initial display info:\n",
> __FUNCTION__));
> +    DbgPrint(TRACE_LEVEL_INFORMATION, ("ACPI/Target Id: %d:%d\n",
> pInfo->AcpiId, pInfo->TargetId));
> +    DbgPrint(TRACE_LEVEL_INFORMATION, ("W/H/Pitch/Color:%d:%d:%d:%d\n",
> pInfo->Width, pInfo->Height, pInfo->Pitch, pInfo->ColorFormat));
> +    DbgPrint(TRACE_LEVEL_INFORMATION, ("Physical address:%I64x\n",
> pInfo->PhysicAddress.QuadPart));
> +
>      Status = RegisterHWInfo(m_pHWDevice->GetId());
>      if (!NT_SUCCESS(Status))
>      {
> @@ -668,6 +674,12 @@ NTSTATUS
> QxlDod::StopDeviceAndReleasePostDisplayOwnership(_In_  D3DDDI_VIDEO_PRE
>      m_pHWDevice->BlackOutScreen(&m_CurrentModes[SourceId]);
>  
>      *pDisplayInfo = m_CurrentModes[SourceId].DispInfo;
> +    m_pHWDevice->GetFinalDisplayInfo(pDisplayInfo);
> +
> +    DbgPrint(TRACE_LEVEL_INFORMATION, ("%s: Final display info:\n",
> __FUNCTION__));
> +    DbgPrint(TRACE_LEVEL_INFORMATION, ("ACPI/Target Id: %d:%d\n",
> pDisplayInfo->AcpiId, pDisplayInfo->TargetId));
> +    DbgPrint(TRACE_LEVEL_INFORMATION, ("W/H/Pitch/Color:%d:%d:%d:%d\n",
> pDisplayInfo->Width, pDisplayInfo->Height, pDisplayInfo->Pitch,
> pDisplayInfo->ColorFormat));
> +    DbgPrint(TRACE_LEVEL_INFORMATION, ("Physical address:%I64x\n",
> pDisplayInfo->PhysicAddress.QuadPart));
>  
>      return StopDevice();
>  }
> @@ -3055,6 +3067,19 @@ NTSTATUS VgaDevice::Escape(_In_ CONST DXGKARG_ESCAPE*
> pEscap)
>      return STATUS_NOT_IMPLEMENTED;
>  }
>  
> +static BOOLEAN IsUefiMode()
> +{
> +    PAGED_CODE();
> +    UNICODE_STRING usName;
> +    GUID guid = {};
> +    ULONG res, len = sizeof(res);
> +    RtlInitUnicodeString(&usName, L"dummy");
> +    NTSTATUS status = ExGetFirmwareEnvironmentVariable(&usName, &guid, &res,
> &len, NULL);
> +    DbgPrint(TRACE_LEVEL_VERBOSE, ("%s, status %X\n", __FUNCTION__,
> status));
> +    // on UEFI system the status is STATUS_VARIABLE_NOT_FOUND
> +    return status != STATUS_NOT_IMPLEMENTED;
> +}
> +
>  QxlDevice::QxlDevice(_In_ QxlDod* pQxlDod)
>  {
>      PAGED_CODE();
> @@ -3068,6 +3093,8 @@ QxlDevice::QxlDevice(_In_ QxlDod* pQxlDod)
>      m_Pending = 0;
>      m_PresentThread = NULL;
>      m_bActive = FALSE;
> +    m_bUefiMode = IsUefiMode();
> +    DbgPrint(TRACE_LEVEL_VERBOSE, ("%s, %s mode\n", __FUNCTION__,
> m_bUefiMode ? "UEFI" : "BIOS"));
>  }
>  
>  QxlDevice::~QxlDevice(void)
> @@ -3331,6 +3358,17 @@ NTSTATUS QxlDevice::SetPowerState(DEVICE_POWER_STATE
> DevicePowerState, DXGK_DISP
>      return STATUS_SUCCESS;
>  }
>  
> +void QxlDevice::GetFinalDisplayInfo(DXGK_DISPLAY_INFORMATION* pDisplayInfo)
> +{
> +    PAGED_CODE();
> +    *pDisplayInfo = m_InitialDisplayInfo;
> +    pDisplayInfo->TargetId = pDisplayInfo->AcpiId = 0;
> +    if (!pDisplayInfo->PhysicAddress.QuadPart)
> +    {
> +        pDisplayInfo->PhysicAddress = m_RamPA;
> +    }
> +}
> +
>  NTSTATUS QxlDevice::HWInit(PCM_RESOURCE_LIST pResList,
>  DXGK_DISPLAY_INFORMATION* pDispInfo)
>  {
>      PAGED_CODE();
> @@ -3530,10 +3568,11 @@ NTSTATUS QxlDevice::QxlInit(DXGK_DISPLAY_INFORMATION*
> pDispInfo)
>          DbgPrint(TRACE_LEVEL_ERROR, ("InitMonitorConfig failed with status
>          0x%X\n", Status));
>          return Status;
>      }
> -    Status = AcquireDisplayInfo(*(pDispInfo));
> +    Status = AcquireDisplayInfo(m_InitialDisplayInfo);
>      if (NT_SUCCESS(Status))
>      {
>          m_bActive = TRUE;
> +        *pDispInfo = m_InitialDisplayInfo;
>          Status = StartPresentThread();
>      }
>      if (!NT_SUCCESS(Status)) {
> @@ -4728,6 +4767,11 @@ NTSTATUS QxlDevice::HWClose(void)
>  {
>      PAGED_CODE();
>      QxlClose();
> +    if (m_bUefiMode)
> +    {
> +        DbgPrint(TRACE_LEVEL_INFORMATION, ("%s: Resetting the device\n",
> __FUNCTION__));
> +        WRITE_PORT_UCHAR((PUCHAR)(m_IoBase + QXL_IO_RESET), 0);
> +    }
>      UnmapMemory();
>      return STATUS_SUCCESS;
>  }
> @@ -5143,6 +5187,7 @@ NTSTATUS
> HwDeviceInterface::AcquireDisplayInfo(DXGK_DISPLAY_INFORMATION& DispInf
>  
>      if (DispInfo.Width == 0)
>      {
> +        DbgPrint(TRACE_LEVEL_WARNING, ("QxlDod::AcquireDisplayInfo has zero
> width!\n"));
>          DispInfo.ColorFormat = D3DDDIFMT_A8R8G8B8;
>          DispInfo.Width = INITIAL_WIDTH;
>          DispInfo.Height = INITIAL_HEIGHT;
> diff --git a/qxldod/QxlDod.h b/qxldod/QxlDod.h
> index eb6b78d..d889b9d 100755
> --- a/qxldod/QxlDod.h
> +++ b/qxldod/QxlDod.h
> @@ -285,6 +285,7 @@ public:
>      QXL_NON_PAGED virtual VOID VSyncInterruptPostProcess(_In_
>      PDXGKRNL_INTERFACE) = 0;
>      virtual NTSTATUS AcquireFrameBuffer(CURRENT_BDD_MODE* pCurrentBddMode) {
>      return STATUS_SUCCESS; }
>      virtual NTSTATUS ReleaseFrameBuffer(CURRENT_BDD_MODE* pCurrentBddMode) {
>      return STATUS_SUCCESS; }
> +    virtual void GetFinalDisplayInfo(DXGK_DISPLAY_INFORMATION* pDisplayInfo)
> {}
>  
>      ULONG GetModeCount(void) const {return m_ModeCount;}
>      PVIDEO_MODE_INFORMATION GetModeInfo(UINT idx) {return &m_ModeInfo[idx];}
> @@ -555,7 +556,8 @@ public:
>      NTSTATUS SetPointerShape(_In_ CONST DXGKARG_SETPOINTERSHAPE*
>      pSetPointerShape);
>      NTSTATUS SetPointerPosition(_In_ CONST DXGKARG_SETPOINTERPOSITION*
>      pSetPointerPosition);
>      NTSTATUS Escape(_In_ CONST DXGKARG_ESCAPE* pEscap);
> -    BOOLEAN IsBIOSCompatible() { return FALSE; }
> +    BOOLEAN IsBIOSCompatible() { return m_bUefiMode; }

So when is UEFI is BIOS compatible and when is BIOS is not BIOS compatible?
It looks a bit odd, I'm sure it works but is confusing.

> +    void GetFinalDisplayInfo(DXGK_DISPLAY_INFORMATION* pDisplayInfo);
>  protected:
>      NTSTATUS GetModeList(DXGK_DISPLAY_INFORMATION* pDispInfo);
>      QXLDrawable *PrepareBltBits (BLT_INFO* pDst,
> @@ -695,6 +697,8 @@ private:
>      uint16_t m_DrawGeneration;
>      HANDLE m_PresentThread;
>      BOOLEAN m_bActive;
> +    BOOLEAN m_bUefiMode;
> +    DXGK_DISPLAY_INFORMATION m_InitialDisplayInfo;
>  };
>  
>  class QxlDod {

Frediano