Hey, not overly familiar with the windows code, but looks reasonable

Reviewed-by: Christophe Fergeau <cferg...@redhat.com>

On Thu, Jan 17, 2019 at 11:08:48AM +0200, Yuri Benditovich wrote:
> 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.benditov...@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; }
> +    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 {
> -- 
> 2.16.1.windows.4
> 
> _______________________________________________
> Spice-devel mailing list
> Spice-devel@lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/spice-devel

Attachment: signature.asc
Description: PGP signature

_______________________________________________
Spice-devel mailing list
Spice-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/spice-devel

Reply via email to