[Spice-devel] [PATCH qxl-win 1/4] miniport: map vram to virtual address only once, #722954
Yonit Halperin
yhalperi at redhat.com
Wed Aug 24 05:05:26 PDT 2011
Drv<Enable/Disbale>Surface calls IOCTL_VIDEO_MAP/UNMAP_VIDEO_MEMORY respectively.
These calls make the miniport map/unmap the vram to/from system space.
The BSOD occurred since with qxl revision 2, the driver assumes that
as long as it is loaded the vram stays (and all the other mem slots) valid
and in the same place. However, the miniport's call to VideoPortMapMemory
(in IOCTL_VIDEO_MAP_VIDEO_MEMORY) is not guaranteed to map the vram to the same
virtual address. As a result, when the driver accessed to an mspace instance
that was initialized with a no longer valid virtual address, we got BSOD.
Since we don't change the size of the vram, I see no point in mapping and
unmapping it more then once.
For qxl revision 3 this problem is not relevant since between DrvAssertMode(disable)
to DrvAssertMode(enable) we clear the pci ram and initialize all
the data structure that are related to it (mspace, caches, etc.)
I didn't remove the calls to IOCTL_VIDEO_MAP/UNMAP_VIDEO_MEMORY
since I think they would be useful for supporting dynamically changing the
size of the memslots according to the current resolution. Which I think is the
real purpose of IOCTL_VIDEO_MAP/UNMAP_VIDEO_MEMORY. Today, the size
of the frame buffer (surface0) is static -> the maximal resolution we support.
---
miniport/qxl.c | 75 ++++++++++++++++++++++---------------------------------
1 files changed, 30 insertions(+), 45 deletions(-)
diff --git a/miniport/qxl.c b/miniport/qxl.c
index 40e8379..7006adf 100644
--- a/miniport/qxl.c
+++ b/miniport/qxl.c
@@ -324,6 +324,11 @@ VP_STATUS InitVRAM(QXLExtension *dev, PVIDEO_ACCESS_RANGE range);
VP_STATUS InitVRAM(QXLExtension *dev, PVIDEO_ACCESS_RANGE range)
{
+ UINT8 *vram_addr = NULL;
+ ULONG vram_mapped_size = range->RangeLength;
+ ULONG io_space = VIDEO_MEMORY_SPACE_MEMORY;
+ VP_STATUS error;
+
PAGED_CODE();
DEBUG_PRINT((dev, 0, "%s\n", __FUNCTION__));
@@ -332,10 +337,24 @@ VP_STATUS InitVRAM(QXLExtension *dev, PVIDEO_ACCESS_RANGE range)
return ERROR_INVALID_DATA;
}
+ if ((error = VideoPortMapMemory(dev, range->RangeStart,
+ &vram_mapped_size,
+ &io_space,
+ &vram_addr))) {
+ DEBUG_PRINT((dev, 0, "%s: map vram failed\n", __FUNCTION__));
+ return error;
+ }
+
+ if (vram_mapped_size < range->RangeLength) {
+ DEBUG_PRINT((dev, 0, "%s: vram shrinked\n", __FUNCTION__));
+ VideoPortUnmapMemory(dev, vram_addr, NULL);
+ return ERROR_NOT_ENOUGH_MEMORY;
+ }
dev->vram_physical = range->RangeStart;
+ dev->vram_start = vram_addr;
dev->vram_size = range->RangeLength;
- DEBUG_PRINT((dev, 0, "%s: OK, vram 0x%lx size %lu\n",
- __FUNCTION__, (ULONG)range->RangeStart.QuadPart, range->RangeLength));
+ DEBUG_PRINT((dev, 0, "%s: OK, vram 0x%lx size %lu vaddr 0x%lx\n", __FUNCTION__,
+ (ULONG)range->RangeStart.QuadPart, range->RangeLength, dev->vram_start));
return NO_ERROR;
}
@@ -577,6 +596,10 @@ void DevExternsionCleanup(QXLExtension *dev)
VideoPortUnmapMemory(dev, dev->ram_start, NULL);
}
+ if (dev->vram_start) {
+ VideoPortUnmapMemory(dev, dev->vram_start, NULL);
+ }
+
if (dev->modes) {
VideoPortFreePool(dev, dev->modes);
}
@@ -966,7 +989,6 @@ BOOLEAN StartIO(PVOID dev_extension, PVIDEO_REQUEST_PACKET packet)
break;
case IOCTL_VIDEO_MAP_VIDEO_MEMORY: {
PVIDEO_MEMORY_INFORMATION mem_info;
- ULONG fb_io_space;
DEBUG_PRINT((dev_ext, 0, "%s: IOCTL_VIDEO_MAP_VIDEO_MEMORY\n", __FUNCTION__));
@@ -976,36 +998,11 @@ BOOLEAN StartIO(PVOID dev_extension, PVIDEO_REQUEST_PACKET packet)
error = ERROR_INSUFFICIENT_BUFFER;
goto err;
}
- mem_info = packet->OutputBuffer;
- mem_info->VideoRamBase =
- ((PVIDEO_MEMORY)(packet->InputBuffer))->RequestedVirtualAddress;
- ASSERT(mem_info->VideoRamBase == NULL);
- mem_info->VideoRamLength = dev_ext->vram_size;
- fb_io_space = VIDEO_MEMORY_SPACE_MEMORY;
-
- if ((error = VideoPortMapMemory(dev_ext, dev_ext->vram_physical,
- &mem_info->VideoRamLength,
- &fb_io_space, &mem_info->VideoRamBase)) != NO_ERROR) {
- DEBUG_PRINT((dev_ext, 0, "%s: map filed\n", __FUNCTION__));
- goto err;
- }
- dev_ext->vram_start = mem_info->VideoRamBase;
- DEBUG_PRINT((dev_ext, 0, "%s: vram size %lu ret size %lu fb vaddr 0x%lx\n",
- __FUNCTION__,
- dev_ext->vram_size,
- mem_info->VideoRamLength,
- mem_info->VideoRamBase));
- if (mem_info->VideoRamLength < dev_ext->vram_size) {
- DEBUG_PRINT((dev_ext, 0, "%s: fb shrink\n", __FUNCTION__));
- VideoPortUnmapMemory(dev_ext, mem_info->VideoRamBase, NULL);
- mem_info->VideoRamBase = NULL;
- mem_info->VideoRamLength = 0;
- error = ERROR_NOT_ENOUGH_MEMORY;
- goto err;
- }
- mem_info->FrameBufferBase = mem_info->VideoRamBase;
- mem_info->FrameBufferLength = mem_info->VideoRamLength;
+ ASSERT(((PVIDEO_MEMORY)(packet->InputBuffer))->RequestedVirtualAddress == NULL);
+ mem_info = packet->OutputBuffer;
+ mem_info->VideoRamBase = mem_info->FrameBufferBase = dev_ext->vram_start;
+ mem_info->VideoRamLength = mem_info->FrameBufferLength = dev_ext->vram_size;
#if 0
#ifdef DBG
DEBUG_PRINT((dev, 0, "%s: zap\n", __FUNCTION__));
@@ -1016,19 +1013,7 @@ BOOLEAN StartIO(PVOID dev_extension, PVIDEO_REQUEST_PACKET packet)
}
break;
case IOCTL_VIDEO_UNMAP_VIDEO_MEMORY: {
- PVOID addr;
-
- DEBUG_PRINT((dev_ext, 0, "%s: IOCTL_VIDEO_UNMAP_VIDEO_MEMORY\n", __FUNCTION__));
-
- if (packet->InputBufferLength < sizeof(VIDEO_MEMORY)) {
- error = ERROR_INSUFFICIENT_BUFFER;
- goto err;
- }
- addr = ((PVIDEO_MEMORY)(packet->InputBuffer))->RequestedVirtualAddress;
- if ((error = VideoPortUnmapMemory(dev_ext, addr, NULL)) != NO_ERROR) {
- DEBUG_PRINT((dev_ext, 0, "%s: unmap failed\n", __FUNCTION__));
- }
- dev_ext->vram_start = NULL;
+ DEBUG_PRINT((dev_ext, 0, "%s: IOCTL_VIDEO_UNMAP_VIDEO_MEMORY (do nothing) \n", __FUNCTION__));
}
break;
case IOCTL_VIDEO_RESET_DEVICE:
--
1.7.4.4
More information about the Spice-devel
mailing list