* [PATCH v2 1/5] Revert "OvmfPkg/QemuVideoDxe: list "UnalignedIoInternal.h" in the INF file"
2018-11-02 3:23 [PATCH v2 0/5] OvmfPkg: simply use the Bochs interface for vmsvga yuchenlin
@ 2018-11-02 3:23 ` yuchenlin
2018-11-06 10:03 ` Laszlo Ersek
2018-11-02 3:23 ` [PATCH v2 2/5] Revert "OvmfPkg/QemuVideoDxe: VMWare SVGA device support" yuchenlin
` (5 subsequent siblings)
6 siblings, 1 reply; 23+ messages in thread
From: yuchenlin @ 2018-11-02 3:23 UTC (permalink / raw)
To: edk2-devel
Cc: jordan.l.justen, lersek, ard.biesheuvel, anthony.perard,
julien.grall, phil, kraxel, philmd, yuchenlin
From: yuchenlin <yuchenlin@synology.com>
This reverts commit b2959e9f1a57279506ca46d56bc424fd7fa6b62a.
The VMWare SVGA display device implemented by Qemu (-vga vmware) uses
an I/O-type BAR which is laid out such that some register offsets are
not aligned to the read/write width with which they are expected to be
accessed. However, we will revert the initialization of VMWare SVGA
device later, we don't need such unaligned I/O.
Contributed-under: TianoCore Contribution Agreement 1.1
Signed-off-by: yuchenlin <yuchenlin@synology.com>
---
OvmfPkg/QemuVideoDxe/QemuVideoDxe.inf | 1 -
1 file changed, 1 deletion(-)
diff --git a/OvmfPkg/QemuVideoDxe/QemuVideoDxe.inf b/OvmfPkg/QemuVideoDxe/QemuVideoDxe.inf
index a04ed537dd..895e6b8dbd 100644
--- a/OvmfPkg/QemuVideoDxe/QemuVideoDxe.inf
+++ b/OvmfPkg/QemuVideoDxe/QemuVideoDxe.inf
@@ -39,7 +39,6 @@
Gop.c
Initialize.c
Qemu.h
- UnalignedIoInternal.h
[Sources.Ia32, Sources.X64]
UnalignedIoGcc.c | GCC
--
2.18.0
^ permalink raw reply related [flat|nested] 23+ messages in thread* Re: [PATCH v2 1/5] Revert "OvmfPkg/QemuVideoDxe: list "UnalignedIoInternal.h" in the INF file"
2018-11-02 3:23 ` [PATCH v2 1/5] Revert "OvmfPkg/QemuVideoDxe: list "UnalignedIoInternal.h" in the INF file" yuchenlin
@ 2018-11-06 10:03 ` Laszlo Ersek
0 siblings, 0 replies; 23+ messages in thread
From: Laszlo Ersek @ 2018-11-06 10:03 UTC (permalink / raw)
To: yuchenlin, edk2-devel; +Cc: phil, jordan.l.justen, anthony.perard
On 11/02/18 04:23, yuchenlin via edk2-devel wrote:
> From: yuchenlin <yuchenlin@synology.com>
>
> This reverts commit b2959e9f1a57279506ca46d56bc424fd7fa6b62a.
>
> The VMWare SVGA display device implemented by Qemu (-vga vmware) uses
> an I/O-type BAR which is laid out such that some register offsets are
> not aligned to the read/write width with which they are expected to be
> accessed. However, we will revert the initialization of VMWare SVGA
> device later, we don't need such unaligned I/O.
>
> Contributed-under: TianoCore Contribution Agreement 1.1
> Signed-off-by: yuchenlin <yuchenlin@synology.com>
> ---
> OvmfPkg/QemuVideoDxe/QemuVideoDxe.inf | 1 -
> 1 file changed, 1 deletion(-)
>
> diff --git a/OvmfPkg/QemuVideoDxe/QemuVideoDxe.inf b/OvmfPkg/QemuVideoDxe/QemuVideoDxe.inf
> index a04ed537dd..895e6b8dbd 100644
> --- a/OvmfPkg/QemuVideoDxe/QemuVideoDxe.inf
> +++ b/OvmfPkg/QemuVideoDxe/QemuVideoDxe.inf
> @@ -39,7 +39,6 @@
> Gop.c
> Initialize.c
> Qemu.h
> - UnalignedIoInternal.h
>
> [Sources.Ia32, Sources.X64]
> UnalignedIoGcc.c | GCC
>
Reviewed-by: Laszlo Ersek <lersek@redhat.com>
^ permalink raw reply [flat|nested] 23+ messages in thread
* [PATCH v2 2/5] Revert "OvmfPkg/QemuVideoDxe: VMWare SVGA device support"
2018-11-02 3:23 [PATCH v2 0/5] OvmfPkg: simply use the Bochs interface for vmsvga yuchenlin
2018-11-02 3:23 ` [PATCH v2 1/5] Revert "OvmfPkg/QemuVideoDxe: list "UnalignedIoInternal.h" in the INF file" yuchenlin
@ 2018-11-02 3:23 ` yuchenlin
2018-11-06 10:36 ` Laszlo Ersek
2018-11-02 3:24 ` [PATCH v2 3/5] Revert "OvmfPkg/QemuVideoDxe: Helper functions for unaligned port I/O." yuchenlin
` (4 subsequent siblings)
6 siblings, 1 reply; 23+ messages in thread
From: yuchenlin @ 2018-11-02 3:23 UTC (permalink / raw)
To: edk2-devel
Cc: jordan.l.justen, lersek, ard.biesheuvel, anthony.perard,
julien.grall, phil, kraxel, philmd, yuchenlin
From: yuchenlin <yuchenlin@synology.com>
This reverts commit c137d95081690d4877fbeb5f1856972e84ac32f2.
The VMWare SVGA model now -- since commit 104bd1dc70 in QEMU --
falls back to stdvga (that is, Bochs) if we don't setup VMWare SVGA
FIFO.
To simplify QemuVideoDxe, we don't intend to implement the VMWare SVGA
FIFO setup feature. It means our current VMW SVGA driver code is
basically dead. To simplify the problem, we will replace the old
VMWare SVGA driver to Bochs interface. It should work on all QEMU
version.
The first step for using Bochs interface is to revert old driver.
Contributed-under: TianoCore Contribution Agreement 1.1
Signed-off-by: yuchenlin <yuchenlin@synology.com>
---
OvmfPkg/QemuVideoDxe/Driver.c | 135 +------------------------
OvmfPkg/QemuVideoDxe/Gop.c | 65 +------------
OvmfPkg/QemuVideoDxe/Initialize.c | 157 ------------------------------
OvmfPkg/QemuVideoDxe/Qemu.h | 29 ------
4 files changed, 7 insertions(+), 379 deletions(-)
diff --git a/OvmfPkg/QemuVideoDxe/Driver.c b/OvmfPkg/QemuVideoDxe/Driver.c
index 73eb2cad05..2304afd1e6 100644
--- a/OvmfPkg/QemuVideoDxe/Driver.c
+++ b/OvmfPkg/QemuVideoDxe/Driver.c
@@ -14,10 +14,8 @@
**/
-#include <IndustryStandard/VmwareSvga.h>
-#include <IndustryStandard/Acpi.h>
#include "Qemu.h"
-#include "UnalignedIoInternal.h"
+#include <IndustryStandard/Acpi.h>
EFI_DRIVER_BINDING_PROTOCOL gQemuVideoDriverBinding = {
QemuVideoControllerDriverSupported,
@@ -71,12 +69,6 @@ QEMU_VIDEO_CARD gQemuVideoCardList[] = {
0x1050,
QEMU_VIDEO_BOCHS_MMIO,
L"QEMU VirtIO VGA"
- },{
- PCI_CLASS_DISPLAY_VGA,
- VMWARE_PCI_VENDOR_ID_VMWARE,
- VMWARE_PCI_DEVICE_ID_VMWARE_SVGA2,
- QEMU_VIDEO_VMWARE_SVGA,
- L"QEMU VMWare SVGA"
},{
0 /* end of list */
}
@@ -264,7 +256,6 @@ QemuVideoControllerDriverStart (
goto ClosePciIo;
}
Private->Variant = Card->Variant;
- Private->FrameBufferVramBarIndex = PCI_BAR_IDX0;
//
// IsQxl is based on the detected Card->Variant, which at a later point might
@@ -339,58 +330,6 @@ QemuVideoControllerDriverStart (
}
}
- //
- // Check if accessing Vmware SVGA interface works
- //
- if (Private->Variant == QEMU_VIDEO_VMWARE_SVGA) {
- EFI_ACPI_ADDRESS_SPACE_DESCRIPTOR *IoDesc;
- UINT32 TargetId;
- UINT32 SvgaIdRead;
-
- IoDesc = NULL;
- Status = Private->PciIo->GetBarAttributes (
- Private->PciIo,
- PCI_BAR_IDX0,
- NULL,
- (VOID**) &IoDesc
- );
- if (EFI_ERROR (Status) ||
- IoDesc->ResType != ACPI_ADDRESS_SPACE_TYPE_IO ||
- IoDesc->AddrRangeMin > MAX_UINT16 + 1 - (VMWARE_SVGA_VALUE_PORT + 4)) {
- if (IoDesc != NULL) {
- FreePool (IoDesc);
- }
- Status = EFI_DEVICE_ERROR;
- goto RestoreAttributes;
- }
- Private->VmwareSvgaBasePort = (UINT16) IoDesc->AddrRangeMin;
- FreePool (IoDesc);
-
- TargetId = VMWARE_SVGA_ID_2;
- while (TRUE) {
- VmwareSvgaWrite (Private, VmwareSvgaRegId, TargetId);
- SvgaIdRead = VmwareSvgaRead (Private, VmwareSvgaRegId);
- if ((SvgaIdRead == TargetId) || (TargetId <= VMWARE_SVGA_ID_0)) {
- break;
- }
- TargetId--;
- }
-
- if (SvgaIdRead != TargetId) {
- DEBUG ((
- DEBUG_ERROR,
- "QemuVideo: QEMU_VIDEO_VMWARE_SVGA ID mismatch "
- "(got 0x%x, base address 0x%x)\n",
- SvgaIdRead,
- Private->VmwareSvgaBasePort
- ));
- Status = EFI_DEVICE_ERROR;
- goto RestoreAttributes;
- }
-
- Private->FrameBufferVramBarIndex = PCI_BAR_IDX1;
- }
-
//
// Get ParentDevicePath
//
@@ -446,9 +385,6 @@ QemuVideoControllerDriverStart (
case QEMU_VIDEO_BOCHS:
Status = QemuVideoBochsModeSetup (Private, IsQxl);
break;
- case QEMU_VIDEO_VMWARE_SVGA:
- Status = QemuVideoVmwareSvgaModeSetup (Private);
- break;
default:
ASSERT (FALSE);
Status = EFI_DEVICE_ERROR;
@@ -510,9 +446,6 @@ DestructQemuVideoGraphics:
FreeModeData:
FreePool (Private->ModeData);
- if (Private->VmwareSvgaModeInfo != NULL) {
- FreePool (Private->VmwareSvgaModeInfo);
- }
UninstallGopDevicePath:
gBS->UninstallProtocolInterface (Private->Handle,
@@ -634,9 +567,6 @@ QemuVideoControllerDriverStop (
);
FreePool (Private->ModeData);
- if (Private->VmwareSvgaModeInfo != NULL) {
- FreePool (Private->VmwareSvgaModeInfo);
- }
gBS->UninstallProtocolInterface (Private->Handle,
&gEfiDevicePathProtocolGuid, Private->GopDevicePath);
FreePool (Private->GopDevicePath);
@@ -834,7 +764,7 @@ ClearScreen (
Private->PciIo->Mem.Write (
Private->PciIo,
EfiPciIoWidthFillUint32,
- Private->FrameBufferVramBarIndex,
+ 0,
0,
0x400000 >> 2,
&Color
@@ -971,38 +901,6 @@ BochsRead (
return Data;
}
-VOID
-VmwareSvgaWrite (
- QEMU_VIDEO_PRIVATE_DATA *Private,
- UINT16 Register,
- UINT32 Value
- )
-{
- UnalignedIoWrite32 (
- Private->VmwareSvgaBasePort + VMWARE_SVGA_INDEX_PORT,
- Register
- );
- UnalignedIoWrite32 (
- Private->VmwareSvgaBasePort + VMWARE_SVGA_VALUE_PORT,
- Value
- );
-}
-
-UINT32
-VmwareSvgaRead (
- QEMU_VIDEO_PRIVATE_DATA *Private,
- UINT16 Register
- )
-{
- UnalignedIoWrite32 (
- Private->VmwareSvgaBasePort + VMWARE_SVGA_INDEX_PORT,
- Register
- );
- return UnalignedIoRead32 (
- Private->VmwareSvgaBasePort + VMWARE_SVGA_VALUE_PORT
- );
-}
-
VOID
VgaOutb (
QEMU_VIDEO_PRIVATE_DATA *Private,
@@ -1057,35 +955,6 @@ InitializeBochsGraphicsMode (
ClearScreen (Private);
}
-VOID
-InitializeVmwareSvgaGraphicsMode (
- QEMU_VIDEO_PRIVATE_DATA *Private,
- QEMU_VIDEO_BOCHS_MODES *ModeData
- )
-{
- UINT32 Capabilities;
-
- VmwareSvgaWrite (Private, VmwareSvgaRegWidth, ModeData->Width);
- VmwareSvgaWrite (Private, VmwareSvgaRegHeight, ModeData->Height);
-
- Capabilities = VmwareSvgaRead (
- Private,
- VmwareSvgaRegCapabilities
- );
- if ((Capabilities & VMWARE_SVGA_CAP_8BIT_EMULATION) != 0) {
- VmwareSvgaWrite (
- Private,
- VmwareSvgaRegBitsPerPixel,
- ModeData->ColorDepth
- );
- }
-
- VmwareSvgaWrite (Private, VmwareSvgaRegEnable, 1);
-
- SetDefaultPalette (Private);
- ClearScreen (Private);
-}
-
EFI_STATUS
EFIAPI
InitializeQemuVideo (
diff --git a/OvmfPkg/QemuVideoDxe/Gop.c b/OvmfPkg/QemuVideoDxe/Gop.c
index c9941ef138..d490fa7a2e 100644
--- a/OvmfPkg/QemuVideoDxe/Gop.c
+++ b/OvmfPkg/QemuVideoDxe/Gop.c
@@ -13,7 +13,6 @@
**/
-#include <IndustryStandard/VmwareSvga.h>
#include "Qemu.h"
STATIC
@@ -79,46 +78,6 @@ QemuVideoCompleteModeData (
return EFI_SUCCESS;
}
-STATIC
-EFI_STATUS
-QemuVideoVmwareSvgaCompleteModeData (
- IN QEMU_VIDEO_PRIVATE_DATA *Private,
- OUT EFI_GRAPHICS_OUTPUT_PROTOCOL_MODE *Mode
- )
-{
- EFI_STATUS Status;
- EFI_GRAPHICS_OUTPUT_MODE_INFORMATION *Info;
- EFI_ACPI_ADDRESS_SPACE_DESCRIPTOR *FrameBufDesc;
- UINT32 BytesPerLine, FbOffset, BytesPerPixel;
-
- Info = Mode->Info;
- CopyMem (Info, &Private->VmwareSvgaModeInfo[Mode->Mode], sizeof (*Info));
- BytesPerPixel = Private->ModeData[Mode->Mode].ColorDepth / 8;
- BytesPerLine = Info->PixelsPerScanLine * BytesPerPixel;
-
- FbOffset = VmwareSvgaRead (Private, VmwareSvgaRegFbOffset);
-
- Status = Private->PciIo->GetBarAttributes (
- Private->PciIo,
- PCI_BAR_IDX1,
- NULL,
- (VOID**) &FrameBufDesc
- );
- if (EFI_ERROR (Status)) {
- return EFI_DEVICE_ERROR;
- }
-
- Mode->FrameBufferBase = FrameBufDesc->AddrRangeMin + FbOffset;
- Mode->FrameBufferSize = BytesPerLine * Info->VerticalResolution;
- Mode->FrameBufferSize = EFI_PAGES_TO_SIZE (
- EFI_SIZE_TO_PAGES (Mode->FrameBufferSize)
- );
-
- FreePool (FrameBufDesc);
- return Status;
-}
-
-
//
// Graphics Output Protocol Member Functions
//
@@ -167,14 +126,10 @@ Routine Description:
*SizeOfInfo = sizeof (EFI_GRAPHICS_OUTPUT_MODE_INFORMATION);
- if (Private->Variant == QEMU_VIDEO_VMWARE_SVGA) {
- CopyMem (*Info, &Private->VmwareSvgaModeInfo[ModeNumber], sizeof (**Info));
- } else {
- ModeData = &Private->ModeData[ModeNumber];
- (*Info)->HorizontalResolution = ModeData->HorizontalResolution;
- (*Info)->VerticalResolution = ModeData->VerticalResolution;
- QemuVideoCompleteModeInfo (ModeData, *Info);
- }
+ ModeData = &Private->ModeData[ModeNumber];
+ (*Info)->HorizontalResolution = ModeData->HorizontalResolution;
+ (*Info)->VerticalResolution = ModeData->VerticalResolution;
+ QemuVideoCompleteModeInfo (ModeData, *Info);
return EFI_SUCCESS;
}
@@ -224,12 +179,6 @@ Routine Description:
case QEMU_VIDEO_BOCHS:
InitializeBochsGraphicsMode (Private, &QemuVideoBochsModes[ModeData->InternalModeIndex]);
break;
- case QEMU_VIDEO_VMWARE_SVGA:
- InitializeVmwareSvgaGraphicsMode (
- Private,
- &QemuVideoBochsModes[ModeData->InternalModeIndex]
- );
- break;
default:
ASSERT (FALSE);
return EFI_DEVICE_ERROR;
@@ -240,11 +189,7 @@ Routine Description:
This->Mode->Info->VerticalResolution = ModeData->VerticalResolution;
This->Mode->SizeOfInfo = sizeof(EFI_GRAPHICS_OUTPUT_MODE_INFORMATION);
- if (Private->Variant == QEMU_VIDEO_VMWARE_SVGA) {
- QemuVideoVmwareSvgaCompleteModeData (Private, This->Mode);
- } else {
- QemuVideoCompleteModeData (Private, This->Mode);
- }
+ QemuVideoCompleteModeData (Private, This->Mode);
//
// Re-initialize the frame buffer configure when mode changes.
diff --git a/OvmfPkg/QemuVideoDxe/Initialize.c b/OvmfPkg/QemuVideoDxe/Initialize.c
index 357124d628..d5d8cfef96 100644
--- a/OvmfPkg/QemuVideoDxe/Initialize.c
+++ b/OvmfPkg/QemuVideoDxe/Initialize.c
@@ -13,7 +13,6 @@
**/
-#include <IndustryStandard/VmwareSvga.h>
#include "Qemu.h"
@@ -347,159 +346,3 @@ QemuVideoBochsModeSetup (
return EFI_SUCCESS;
}
-EFI_STATUS
-QemuVideoVmwareSvgaModeSetup (
- QEMU_VIDEO_PRIVATE_DATA *Private
- )
-{
- EFI_STATUS Status;
- UINT32 FbSize;
- UINT32 MaxWidth, MaxHeight;
- UINT32 Capabilities;
- UINT32 BitsPerPixel;
- UINT32 Index;
- QEMU_VIDEO_MODE_DATA *ModeData;
- QEMU_VIDEO_BOCHS_MODES *VideoMode;
- EFI_GRAPHICS_OUTPUT_MODE_INFORMATION *ModeInfo;
-
- VmwareSvgaWrite (Private, VmwareSvgaRegEnable, 0);
-
- Private->ModeData =
- AllocatePool (sizeof (Private->ModeData[0]) * QEMU_VIDEO_BOCHS_MODE_COUNT);
- if (Private->ModeData == NULL) {
- Status = EFI_OUT_OF_RESOURCES;
- goto ModeDataAllocError;
- }
-
- Private->VmwareSvgaModeInfo =
- AllocatePool (
- sizeof (Private->VmwareSvgaModeInfo[0]) * QEMU_VIDEO_BOCHS_MODE_COUNT
- );
- if (Private->VmwareSvgaModeInfo == NULL) {
- Status = EFI_OUT_OF_RESOURCES;
- goto ModeInfoAllocError;
- }
-
- FbSize = VmwareSvgaRead (Private, VmwareSvgaRegFbSize);
- MaxWidth = VmwareSvgaRead (Private, VmwareSvgaRegMaxWidth);
- MaxHeight = VmwareSvgaRead (Private, VmwareSvgaRegMaxHeight);
- Capabilities = VmwareSvgaRead (Private, VmwareSvgaRegCapabilities);
- if ((Capabilities & VMWARE_SVGA_CAP_8BIT_EMULATION) != 0) {
- BitsPerPixel = VmwareSvgaRead (
- Private,
- VmwareSvgaRegHostBitsPerPixel
- );
- VmwareSvgaWrite (
- Private,
- VmwareSvgaRegBitsPerPixel,
- BitsPerPixel
- );
- } else {
- BitsPerPixel = VmwareSvgaRead (
- Private,
- VmwareSvgaRegBitsPerPixel
- );
- }
-
- if (FbSize == 0 ||
- MaxWidth == 0 ||
- MaxHeight == 0 ||
- BitsPerPixel == 0 ||
- BitsPerPixel % 8 != 0) {
- Status = EFI_DEVICE_ERROR;
- goto Rollback;
- }
-
- ModeData = Private->ModeData;
- ModeInfo = Private->VmwareSvgaModeInfo;
- VideoMode = &QemuVideoBochsModes[0];
- for (Index = 0; Index < QEMU_VIDEO_BOCHS_MODE_COUNT; Index++) {
- UINTN RequiredFbSize;
-
- RequiredFbSize = (UINTN) VideoMode->Width * VideoMode->Height *
- (BitsPerPixel / 8);
- if (RequiredFbSize <= FbSize &&
- VideoMode->Width <= MaxWidth &&
- VideoMode->Height <= MaxHeight) {
- UINT32 BytesPerLine;
- UINT32 RedMask, GreenMask, BlueMask, PixelMask;
-
- VmwareSvgaWrite (
- Private,
- VmwareSvgaRegWidth,
- VideoMode->Width
- );
- VmwareSvgaWrite (
- Private,
- VmwareSvgaRegHeight,
- VideoMode->Height
- );
-
- ModeData->InternalModeIndex = Index;
- ModeData->HorizontalResolution = VideoMode->Width;
- ModeData->VerticalResolution = VideoMode->Height;
- ModeData->ColorDepth = BitsPerPixel;
-
- //
- // Setting VmwareSvgaRegWidth/VmwareSvgaRegHeight actually changes
- // the device's display mode, so we save all properties of each mode up
- // front to avoid inadvertent mode changes later.
- //
- ModeInfo->Version = 0;
- ModeInfo->HorizontalResolution = ModeData->HorizontalResolution;
- ModeInfo->VerticalResolution = ModeData->VerticalResolution;
-
- ModeInfo->PixelFormat = PixelBitMask;
-
- RedMask = VmwareSvgaRead (Private, VmwareSvgaRegRedMask);
- ModeInfo->PixelInformation.RedMask = RedMask;
-
- GreenMask = VmwareSvgaRead (Private, VmwareSvgaRegGreenMask);
- ModeInfo->PixelInformation.GreenMask = GreenMask;
-
- BlueMask = VmwareSvgaRead (Private, VmwareSvgaRegBlueMask);
- ModeInfo->PixelInformation.BlueMask = BlueMask;
-
- //
- // Reserved mask is whatever bits in the pixel not containing RGB data,
- // so start with binary 1s for every bit in the pixel, then mask off
- // bits already used for RGB. Special case 32 to avoid undefined
- // behaviour in the shift.
- //
- if (BitsPerPixel == 32) {
- if (BlueMask == 0xff && GreenMask == 0xff00 && RedMask == 0xff0000) {
- ModeInfo->PixelFormat = PixelBlueGreenRedReserved8BitPerColor;
- } else if (BlueMask == 0xff0000 &&
- GreenMask == 0xff00 &&
- RedMask == 0xff) {
- ModeInfo->PixelFormat = PixelRedGreenBlueReserved8BitPerColor;
- }
- PixelMask = MAX_UINT32;
- } else {
- PixelMask = (1u << BitsPerPixel) - 1;
- }
- ModeInfo->PixelInformation.ReservedMask =
- PixelMask & ~(RedMask | GreenMask | BlueMask);
-
- BytesPerLine = VmwareSvgaRead (Private, VmwareSvgaRegBytesPerLine);
- ModeInfo->PixelsPerScanLine = BytesPerLine / (BitsPerPixel / 8);
-
- ModeData++;
- ModeInfo++;
- }
- VideoMode++;
- }
- Private->MaxMode = ModeData - Private->ModeData;
- return EFI_SUCCESS;
-
-Rollback:
- FreePool (Private->VmwareSvgaModeInfo);
- Private->VmwareSvgaModeInfo = NULL;
-
-ModeInfoAllocError:
- FreePool (Private->ModeData);
- Private->ModeData = NULL;
-
-ModeDataAllocError:
- return Status;
-}
diff --git a/OvmfPkg/QemuVideoDxe/Qemu.h b/OvmfPkg/QemuVideoDxe/Qemu.h
index bc49f867a4..d7da761705 100644
--- a/OvmfPkg/QemuVideoDxe/Qemu.h
+++ b/OvmfPkg/QemuVideoDxe/Qemu.h
@@ -92,7 +92,6 @@ typedef enum {
QEMU_VIDEO_CIRRUS_5446,
QEMU_VIDEO_BOCHS,
QEMU_VIDEO_BOCHS_MMIO,
- QEMU_VIDEO_VMWARE_SVGA,
} QEMU_VIDEO_VARIANT;
typedef struct {
@@ -117,13 +116,10 @@ typedef struct {
//
UINTN MaxMode;
QEMU_VIDEO_MODE_DATA *ModeData;
- EFI_GRAPHICS_OUTPUT_MODE_INFORMATION *VmwareSvgaModeInfo;
QEMU_VIDEO_VARIANT Variant;
FRAME_BUFFER_CONFIGURE *FrameBufferBltConfigure;
UINTN FrameBufferBltConfigureSize;
- UINT8 FrameBufferVramBarIndex;
- UINT16 VmwareSvgaBasePort;
} QEMU_VIDEO_PRIVATE_DATA;
///
@@ -507,34 +503,9 @@ QemuVideoBochsModeSetup (
BOOLEAN IsQxl
);
-EFI_STATUS
-QemuVideoVmwareSvgaModeSetup (
- QEMU_VIDEO_PRIVATE_DATA *Private
- );
-
VOID
InstallVbeShim (
IN CONST CHAR16 *CardName,
IN EFI_PHYSICAL_ADDRESS FrameBufferBase
);
-
-VOID
-VmwareSvgaWrite (
- QEMU_VIDEO_PRIVATE_DATA *Private,
- UINT16 Register,
- UINT32 Value
- );
-
-UINT32
-VmwareSvgaRead (
- QEMU_VIDEO_PRIVATE_DATA *Private,
- UINT16 Register
- );
-
-VOID
-InitializeVmwareSvgaGraphicsMode (
- QEMU_VIDEO_PRIVATE_DATA *Private,
- QEMU_VIDEO_BOCHS_MODES *ModeData
- );
-
#endif
--
2.18.0
^ permalink raw reply related [flat|nested] 23+ messages in thread* Re: [PATCH v2 2/5] Revert "OvmfPkg/QemuVideoDxe: VMWare SVGA device support"
2018-11-02 3:23 ` [PATCH v2 2/5] Revert "OvmfPkg/QemuVideoDxe: VMWare SVGA device support" yuchenlin
@ 2018-11-06 10:36 ` Laszlo Ersek
0 siblings, 0 replies; 23+ messages in thread
From: Laszlo Ersek @ 2018-11-06 10:36 UTC (permalink / raw)
To: yuchenlin, edk2-devel; +Cc: phil, jordan.l.justen, anthony.perard
On 11/02/18 04:23, yuchenlin via edk2-devel wrote:
> From: yuchenlin <yuchenlin@synology.com>
>
> This reverts commit c137d95081690d4877fbeb5f1856972e84ac32f2.
>
> The VMWare SVGA model now -- since commit 104bd1dc70 in QEMU --
> falls back to stdvga (that is, Bochs) if we don't setup VMWare SVGA
> FIFO.
>
> To simplify QemuVideoDxe, we don't intend to implement the VMWare SVGA
> FIFO setup feature. It means our current VMW SVGA driver code is
> basically dead. To simplify the problem, we will replace the old
> VMWare SVGA driver to Bochs interface. It should work on all QEMU
> version.
>
> The first step for using Bochs interface is to revert old driver.
>
> Contributed-under: TianoCore Contribution Agreement 1.1
> Signed-off-by: yuchenlin <yuchenlin@synology.com>
> ---
> OvmfPkg/QemuVideoDxe/Driver.c | 135 +------------------------
> OvmfPkg/QemuVideoDxe/Gop.c | 65 +------------
> OvmfPkg/QemuVideoDxe/Initialize.c | 157 ------------------------------
> OvmfPkg/QemuVideoDxe/Qemu.h | 29 ------
> 4 files changed, 7 insertions(+), 379 deletions(-)
I've reviewed this patch as follows:
- I determined that c137d95081690d4877fbeb5f1856972e84ac32f2 was indeed
the commit to revert at this point.
- I applied your series, on top of current master (@ 62ea70e31285),
using local branch name "yuchenlin_v2". Then I checked whether the state
just before c137d9508169 matched the state after this revert -- all
differences would have to be unrelated to the feature in question. My
command was:
git diff --color \
c137d95081690d4877fbeb5f1856972e84ac32f2^..yuchenlin_v2~3 -- \
OvmfPkg/QemuVideoDxe/ \
OvmfPkg/Include/IndustryStandard/
The differences are indeed orthogonal, so this looks like a valid revert.
In addition, the patch itself looks good, plus OVMF continues to build
fine after this patch (for bisectability's sake).
Reviewed-by: Laszlo Ersek <lersek@redhat.com>
Thanks
Laszlo
^ permalink raw reply [flat|nested] 23+ messages in thread
* [PATCH v2 3/5] Revert "OvmfPkg/QemuVideoDxe: Helper functions for unaligned port I/O."
2018-11-02 3:23 [PATCH v2 0/5] OvmfPkg: simply use the Bochs interface for vmsvga yuchenlin
2018-11-02 3:23 ` [PATCH v2 1/5] Revert "OvmfPkg/QemuVideoDxe: list "UnalignedIoInternal.h" in the INF file" yuchenlin
2018-11-02 3:23 ` [PATCH v2 2/5] Revert "OvmfPkg/QemuVideoDxe: VMWare SVGA device support" yuchenlin
@ 2018-11-02 3:24 ` yuchenlin
2018-11-06 10:44 ` Laszlo Ersek
2018-11-02 3:24 ` [PATCH v2 4/5] Revert "OvmfPkg: VMWare SVGA display device register definitions" yuchenlin
` (3 subsequent siblings)
6 siblings, 1 reply; 23+ messages in thread
From: yuchenlin @ 2018-11-02 3:24 UTC (permalink / raw)
To: edk2-devel
Cc: jordan.l.justen, lersek, ard.biesheuvel, anthony.perard,
julien.grall, phil, kraxel, philmd, yuchenlin
From: yuchenlin <yuchenlin@synology.com>
This reverts commit 05a5379458725234de8a05780fcb5da2c12680e4.
The VMWare SVGA display device implemented by Qemu (-vga vmware) uses
an I/O-type BAR which is laid out such that some register offsets are
not aligned to the read/write width with which they are expected to be
accessed. However, we reverted the initialization of VMWare SVGA device,
we don't need such unaligned I/O.
Contributed-under: TianoCore Contribution Agreement 1.1
Signed-off-by: yuchenlin <yuchenlin@synology.com>
---
OvmfPkg/QemuVideoDxe/QemuVideoDxe.inf | 6 --
OvmfPkg/QemuVideoDxe/UnalignedIoGcc.c | 70 ----------------
OvmfPkg/QemuVideoDxe/UnalignedIoIcc.c | 80 -------------------
OvmfPkg/QemuVideoDxe/UnalignedIoInternal.h | 59 --------------
OvmfPkg/QemuVideoDxe/UnalignedIoMsc.c | 78 ------------------
OvmfPkg/QemuVideoDxe/UnalignedIoUnsupported.c | 66 ---------------
6 files changed, 359 deletions(-)
delete mode 100644 OvmfPkg/QemuVideoDxe/UnalignedIoGcc.c
delete mode 100644 OvmfPkg/QemuVideoDxe/UnalignedIoIcc.c
delete mode 100644 OvmfPkg/QemuVideoDxe/UnalignedIoInternal.h
delete mode 100644 OvmfPkg/QemuVideoDxe/UnalignedIoMsc.c
delete mode 100644 OvmfPkg/QemuVideoDxe/UnalignedIoUnsupported.c
diff --git a/OvmfPkg/QemuVideoDxe/QemuVideoDxe.inf b/OvmfPkg/QemuVideoDxe/QemuVideoDxe.inf
index 895e6b8dbd..e5575622dc 100644
--- a/OvmfPkg/QemuVideoDxe/QemuVideoDxe.inf
+++ b/OvmfPkg/QemuVideoDxe/QemuVideoDxe.inf
@@ -41,15 +41,9 @@
Qemu.h
[Sources.Ia32, Sources.X64]
- UnalignedIoGcc.c | GCC
- UnalignedIoIcc.c | INTEL
- UnalignedIoMsc.c | MSFT
VbeShim.c
VbeShim.h
-[Sources.EBC]
- UnalignedIoUnsupported.c
-
[Packages]
MdePkg/MdePkg.dec
MdeModulePkg/MdeModulePkg.dec
diff --git a/OvmfPkg/QemuVideoDxe/UnalignedIoGcc.c b/OvmfPkg/QemuVideoDxe/UnalignedIoGcc.c
deleted file mode 100644
index 105d55d3b9..0000000000
--- a/OvmfPkg/QemuVideoDxe/UnalignedIoGcc.c
+++ /dev/null
@@ -1,70 +0,0 @@
-/** @file
- Unaligned Port I/O. This file has compiler specifics for GCC as there is no
- ANSI C standard for doing IO.
-
- Based on IoLibGcc.c.
-
- Copyright (c) 2006 - 2010, Intel Corporation. All rights reserved.<BR>
- This program and the accompanying materials are licensed and made available
- under the terms and conditions of the BSD License which accompanies this
- distribution. The full text of the license may be found at
- http://opensource.org/licenses/bsd-license.php.
-
- THE PROGRAM IS DISTRIBUTED UNDER THE BSD LICENSE ON AN "AS IS" BASIS,
- WITHOUT WARRANTIES OR REPRESENTATIONS OF ANY KIND, EITHER EXPRESS OR IMPLIED.
-
-**/
-
-
-#include "UnalignedIoInternal.h"
-
-/**
- Performs a 32-bit write to the specified, possibly unaligned I/O-type
- address.
-
- Writes the 32-bit I/O port specified by Port with the value specified by
- Value and returns Value. This function must guarantee that all I/O read and
- write operations are serialized.
-
- If 32-bit unaligned I/O port operations are not supported, then ASSERT().
-
- @param[in] Port I/O port address
- @param[in] Value 32-bit word to write
-
- @return The value written to the I/O port.
-
-**/
-UINT32
-UnalignedIoWrite32 (
- IN UINTN Port,
- IN UINT32 Value
- )
-{
- __asm__ __volatile__ ( "outl %0, %1" : : "a" (Value), "d" ((UINT16)Port) );
- return Value;
-}
-
-/**
- Reads a 32-bit word from the specified, possibly unaligned I/O-type address.
-
- Reads the 32-bit I/O port specified by Port. The 32-bit read value is
- returned. This function must guarantee that all I/O read and write operations
- are serialized.
-
- If 32-bit unaligned I/O port operations are not supported, then ASSERT().
-
- @param[in] Port The I/O port to read.
-
- @return The value read.
-
-**/
-UINT32
-UnalignedIoRead32 (
- IN UINTN Port
- )
-{
- UINT32 Data;
- __asm__ __volatile__ ( "inl %1, %0" : "=a" (Data) : "d" ((UINT16)Port) );
- return Data;
-}
-
diff --git a/OvmfPkg/QemuVideoDxe/UnalignedIoIcc.c b/OvmfPkg/QemuVideoDxe/UnalignedIoIcc.c
deleted file mode 100644
index 79f3e446dd..0000000000
--- a/OvmfPkg/QemuVideoDxe/UnalignedIoIcc.c
+++ /dev/null
@@ -1,80 +0,0 @@
-/** @file
- Unaligned port I/O. This file has compiler specifics for ICC as there
- is no ANSI C standard for doing IO.
-
- Based on IoLibIcc.c.
-
- Copyright (c) 2006 - 2008, Intel Corporation. All rights reserved.<BR>
- This program and the accompanying materials are licensed and made available
- under the terms and conditions of the BSD License which accompanies this
- distribution. The full text of the license may be found at
- http://opensource.org/licenses/bsd-license.php.
-
- THE PROGRAM IS DISTRIBUTED UNDER THE BSD LICENSE ON AN "AS IS" BASIS,
- WITHOUT WARRANTIES OR REPRESENTATIONS OF ANY KIND, EITHER EXPRESS OR IMPLIED.
-
-**/
-
-
-#include "UnalignedIoInternal.h"
-
-/**
- Performs a 32-bit write to the specified, possibly unaligned I/O-type
- address.
-
- Writes the 32-bit I/O port specified by Port with the value specified by
- Value and returns Value. This function must guarantee that all I/O read and
- write operations are serialized.
-
- If 32-bit unaligned I/O port operations are not supported, then ASSERT().
-
- @param[in] Port I/O port address
- @param[in] Value 32-bit word to write
-
- @return The value written to the I/O port.
-
-**/
-UINT32
-UnalignedIoWrite32 (
- IN UINTN Port,
- IN UINT32 Value
- )
-{
- __asm {
- mov eax, dword ptr [Value]
- mov dx, word ptr [Port]
- out dx, eax
- }
-
- return Value;
-}
-
-/**
- Reads a 32-bit word from the specified, possibly unaligned I/O-type address.
-
- Reads the 32-bit I/O port specified by Port. The 32-bit read value is
- returned. This function must guarantee that all I/O read and write operations
- are serialized.
-
- If 32-bit unaligned I/O port operations are not supported, then ASSERT().
-
- @param[in] Port The I/O port to read.
-
- @return The value read.
-
-**/
-UINT32
-UnalignedIoRead32 (
- IN UINTN Port
- )
-{
- UINT32 Data;
-
- __asm {
- mov dx, word ptr [Port]
- in eax, dx
- mov dword ptr [Data], eax
- }
-
- return Data;
-}
diff --git a/OvmfPkg/QemuVideoDxe/UnalignedIoInternal.h b/OvmfPkg/QemuVideoDxe/UnalignedIoInternal.h
deleted file mode 100644
index 234de6c21b..0000000000
--- a/OvmfPkg/QemuVideoDxe/UnalignedIoInternal.h
+++ /dev/null
@@ -1,59 +0,0 @@
-/** @file
- Unaligned port I/O, with implementations for various x86 compilers and a
- dummy for platforms which do not support unaligned port I/O.
-
- Copyright (c) 2017, Phil Dennis-Jordan.<BR>
- This program and the accompanying materials are licensed and made available
- under the terms and conditions of the BSD License which accompanies this
- distribution. The full text of the license may be found at
- http://opensource.org/licenses/bsd-license.php.
-
- THE PROGRAM IS DISTRIBUTED UNDER THE BSD LICENSE ON AN "AS IS" BASIS,
- WITHOUT WARRANTIES OR REPRESENTATIONS OF ANY KIND, EITHER EXPRESS OR IMPLIED.
-
-**/
-
-#ifndef _UNALIGNED_IO_INTERNAL_H_
-#define _UNALIGNED_IO_INTERNAL_H_
-
-/**
- Performs a 32-bit write to the specified, possibly unaligned I/O-type address.
-
- Writes the 32-bit I/O port specified by Port with the value specified by Value
- and returns Value. This function must guarantee that all I/O read and write
- operations are serialized.
-
- If 32-bit unaligned I/O port operations are not supported, then ASSERT().
-
- @param[in] Port I/O port address
- @param[in] Value 32-bit word to write
-
- @return The value written to the I/O port.
-
-**/
-UINT32
-UnalignedIoWrite32 (
- IN UINTN Port,
- IN UINT32 Value
- );
-
-/**
- Reads a 32-bit word from the specified, possibly unaligned I/O-type address.
-
- Reads the 32-bit I/O port specified by Port. The 32-bit read value is
- returned. This function must guarantee that all I/O read and write operations
- are serialized.
-
- If 32-bit unaligned I/O port operations are not supported, then ASSERT().
-
- @param[in] Port The I/O port to read.
-
- @return The value read.
-
-**/
-UINT32
-UnalignedIoRead32 (
- IN UINTN Port
- );
-
-#endif
diff --git a/OvmfPkg/QemuVideoDxe/UnalignedIoMsc.c b/OvmfPkg/QemuVideoDxe/UnalignedIoMsc.c
deleted file mode 100644
index a466baee84..0000000000
--- a/OvmfPkg/QemuVideoDxe/UnalignedIoMsc.c
+++ /dev/null
@@ -1,78 +0,0 @@
-/** @file
- Unaligned port I/O. This file has compiler specifics for Microsoft C as there
- is no ANSI C standard for doing IO.
-
- Based on IoLibMsc.c
-
- Copyright (c) 2006 - 2010, Intel Corporation. All rights reserved.<BR>
- This program and the accompanying materials are licensed and made available
- under the terms and conditions of the BSD License which accompanies this
- distribution. The full text of the license may be found at
- http://opensource.org/licenses/bsd-license.php.
-
- THE PROGRAM IS DISTRIBUTED UNDER THE BSD LICENSE ON AN "AS IS" BASIS,
- WITHOUT WARRANTIES OR REPRESENTATIONS OF ANY KIND, EITHER EXPRESS OR IMPLIED.
-
-**/
-
-
-#include "UnalignedIoInternal.h"
-
-unsigned long _inpd (unsigned short port);
-unsigned long _outpd (unsigned short port, unsigned long dataword );
-void _ReadWriteBarrier (void);
-
-/**
- Performs a 32-bit write to the specified, possibly unaligned I/O-type
- address.
-
- Writes the 32-bit I/O port specified by Port with the value specified by
- Value and returns Value. This function must guarantee that all I/O read and
- write operations are serialized.
-
- If 32-bit unaligned I/O port operations are not supported, then ASSERT().
-
- @param[in] Port I/O port address
- @param[in] Value 32-bit word to write
-
- @return The value written to the I/O port.
-
-**/
-UINT32
-UnalignedIoWrite32 (
- IN UINTN Port,
- IN UINT32 Value
- )
-{
- _ReadWriteBarrier ();
- _outpd ((UINT16)Port, Value);
- _ReadWriteBarrier ();
- return Value;
-}
-
-/**
- Reads a 32-bit word from the specified, possibly unaligned I/O-type address.
-
- Reads the 32-bit I/O port specified by Port. The 32-bit read value is
- returned. This function must guarantee that all I/O read and write operations
- are serialized.
-
- If 32-bit unaligned I/O port operations are not supported, then ASSERT().
-
- @param[in] Port The I/O port to read.
-
- @return The value read.
-
-**/
-UINT32
-UnalignedIoRead32 (
- IN UINTN Port
- )
-{
- UINT32 Value;
-
- _ReadWriteBarrier ();
- Value = _inpd ((UINT16)Port);
- _ReadWriteBarrier ();
- return Value;
-}
diff --git a/OvmfPkg/QemuVideoDxe/UnalignedIoUnsupported.c b/OvmfPkg/QemuVideoDxe/UnalignedIoUnsupported.c
deleted file mode 100644
index 57560ab38f..0000000000
--- a/OvmfPkg/QemuVideoDxe/UnalignedIoUnsupported.c
+++ /dev/null
@@ -1,66 +0,0 @@
-/** @file
- Unaligned port I/O dummy implementation for platforms which do not support it.
-
- Copyright (c) 2017, Phil Dennis-Jordan.<BR>
- This program and the accompanying materials are licensed and made available
- under the terms and conditions of the BSD License which accompanies this
- distribution. The full text of the license may be found at
- http://opensource.org/licenses/bsd-license.php.
-
- THE PROGRAM IS DISTRIBUTED UNDER THE BSD LICENSE ON AN "AS IS" BASIS,
- WITHOUT WARRANTIES OR REPRESENTATIONS OF ANY KIND, EITHER EXPRESS OR IMPLIED.
-
-**/
-
-
-#include <Library/DebugLib.h>
-#include "UnalignedIoInternal.h"
-
-/**
- Performs a 32-bit write to the specified, possibly unaligned I/O-type
- address.
-
- Writes the 32-bit I/O port specified by Port with the value specified by
- Value and returns Value. This function must guarantee that all I/O read and
- write operations are serialized.
-
- If 32-bit unaligned I/O port operations are not supported, then ASSERT().
-
- @param[in] Port I/O port address
- @param[in] Value 32-bit word to write
-
- @return The value written to the I/O port.
-
-**/
-UINT32
-UnalignedIoWrite32 (
- IN UINTN Port,
- IN UINT32 Value
- )
-{
- ASSERT (FALSE);
- return Value;
-}
-
-/**
- Reads a 32-bit word from the specified, possibly unaligned I/O-type address.
-
- Reads the 32-bit I/O port specified by Port. The 32-bit read value is
- returned. This function must guarantee that all I/O read and write operations
- are serialized.
-
- If 32-bit unaligned I/O port operations are not supported, then ASSERT().
-
- @param[in] Port The I/O port to read.
-
- @return The value read.
-
-**/
-UINT32
-UnalignedIoRead32 (
- IN UINTN Port
- )
-{
- ASSERT (FALSE);
- return 0;
-}
--
2.18.0
^ permalink raw reply related [flat|nested] 23+ messages in thread* Re: [PATCH v2 3/5] Revert "OvmfPkg/QemuVideoDxe: Helper functions for unaligned port I/O."
2018-11-02 3:24 ` [PATCH v2 3/5] Revert "OvmfPkg/QemuVideoDxe: Helper functions for unaligned port I/O." yuchenlin
@ 2018-11-06 10:44 ` Laszlo Ersek
0 siblings, 0 replies; 23+ messages in thread
From: Laszlo Ersek @ 2018-11-06 10:44 UTC (permalink / raw)
To: yuchenlin, edk2-devel; +Cc: phil, jordan.l.justen, anthony.perard
On 11/02/18 04:24, yuchenlin via edk2-devel wrote:
> From: yuchenlin <yuchenlin@synology.com>
>
> This reverts commit 05a5379458725234de8a05780fcb5da2c12680e4.
>
> The VMWare SVGA display device implemented by Qemu (-vga vmware) uses
> an I/O-type BAR which is laid out such that some register offsets are
> not aligned to the read/write width with which they are expected to be
> accessed. However, we reverted the initialization of VMWare SVGA device,
> we don't need such unaligned I/O.
>
> Contributed-under: TianoCore Contribution Agreement 1.1
> Signed-off-by: yuchenlin <yuchenlin@synology.com>
> ---
> OvmfPkg/QemuVideoDxe/QemuVideoDxe.inf | 6 --
> OvmfPkg/QemuVideoDxe/UnalignedIoGcc.c | 70 ----------------
> OvmfPkg/QemuVideoDxe/UnalignedIoIcc.c | 80 -------------------
> OvmfPkg/QemuVideoDxe/UnalignedIoInternal.h | 59 --------------
> OvmfPkg/QemuVideoDxe/UnalignedIoMsc.c | 78 ------------------
> OvmfPkg/QemuVideoDxe/UnalignedIoUnsupported.c | 66 ---------------
> 6 files changed, 359 deletions(-)
> delete mode 100644 OvmfPkg/QemuVideoDxe/UnalignedIoGcc.c
> delete mode 100644 OvmfPkg/QemuVideoDxe/UnalignedIoIcc.c
> delete mode 100644 OvmfPkg/QemuVideoDxe/UnalignedIoInternal.h
> delete mode 100644 OvmfPkg/QemuVideoDxe/UnalignedIoMsc.c
> delete mode 100644 OvmfPkg/QemuVideoDxe/UnalignedIoUnsupported.c
Checked this the same way as patch v2 2/5.
Reviewed-by: Laszlo Ersek <lersek@redhat.com>
^ permalink raw reply [flat|nested] 23+ messages in thread
* [PATCH v2 4/5] Revert "OvmfPkg: VMWare SVGA display device register definitions"
2018-11-02 3:23 [PATCH v2 0/5] OvmfPkg: simply use the Bochs interface for vmsvga yuchenlin
` (2 preceding siblings ...)
2018-11-02 3:24 ` [PATCH v2 3/5] Revert "OvmfPkg/QemuVideoDxe: Helper functions for unaligned port I/O." yuchenlin
@ 2018-11-02 3:24 ` yuchenlin
2018-11-06 10:48 ` Laszlo Ersek
2018-11-02 3:24 ` [PATCH v2 5/5] OvmfPkg: simply use the Bochs interface for vmsvga yuchenlin
` (2 subsequent siblings)
6 siblings, 1 reply; 23+ messages in thread
From: yuchenlin @ 2018-11-02 3:24 UTC (permalink / raw)
To: edk2-devel
Cc: jordan.l.justen, lersek, ard.biesheuvel, anthony.perard,
julien.grall, phil, kraxel, philmd, yuchenlin
From: yuchenlin <yuchenlin@synology.com>
This reverts commit 9bcca53fe466cdff397578328d9d87d257aba493.
We reverted VMWare SVGA driver. We don't need these definitions too.
Contributed-under: TianoCore Contribution Agreement 1.1
Signed-off-by: yuchenlin <yuchenlin@synology.com>
---
OvmfPkg/Include/IndustryStandard/VmwareSvga.h | 104 ------------------
1 file changed, 104 deletions(-)
delete mode 100644 OvmfPkg/Include/IndustryStandard/VmwareSvga.h
diff --git a/OvmfPkg/Include/IndustryStandard/VmwareSvga.h b/OvmfPkg/Include/IndustryStandard/VmwareSvga.h
deleted file mode 100644
index 693d44bab6..0000000000
--- a/OvmfPkg/Include/IndustryStandard/VmwareSvga.h
+++ /dev/null
@@ -1,104 +0,0 @@
-/** @file
-
- Macro and enum definitions of a subset of port numbers, register identifiers
- and values required for driving the VMWare SVGA virtual display adapter,
- also implemented by Qemu.
-
- This file's contents was extracted from file lib/vmware/svga_reg.h in commit
- 329dd537456f93a806841ec8a8213aed11395def of VMWare's vmware-svga repository:
- git://git.code.sf.net/p/vmware-svga/git
-
-
- Copyright 1998-2009 VMware, Inc. All rights reserved.
- Portions Copyright 2017 Phil Dennis-Jordan <phil@philjordan.eu>
-
- Permission is hereby granted, free of charge, to any person
- obtaining a copy of this software and associated documentation
- files (the "Software"), to deal in the Software without
- restriction, including without limitation the rights to use, copy,
- modify, merge, publish, distribute, sublicense, and/or sell copies
- of the Software, and to permit persons to whom the Software is
- furnished to do so, subject to the following conditions:
-
- The above copyright notice and this permission notice shall be
- included in all copies or substantial portions of the Software.
-
- THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND,
- EXPRESS OR IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF
- MERCHANTABILITY, FITNESS FOR A PARTICULAR PURPOSE AND
- NONINFRINGEMENT. IN NO EVENT SHALL THE AUTHORS OR COPYRIGHT HOLDERS
- BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER LIABILITY, WHETHER IN AN
- ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING FROM, OUT OF OR IN
- CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS IN THE
- SOFTWARE.
-
-**/
-
-#ifndef _VMWARE_SVGA_H_
-#define _VMWARE_SVGA_H_
-
-#include <Base.h>
-
-//
-// IDs for recognising the device
-//
-#define VMWARE_PCI_VENDOR_ID_VMWARE 0x15AD
-#define VMWARE_PCI_DEVICE_ID_VMWARE_SVGA2 0x0405
-
-//
-// I/O port BAR offsets for register selection and read/write.
-//
-// The register index is written to the 32-bit index port, followed by a 32-bit
-// read or write on the value port to read or set that register's contents.
-//
-#define VMWARE_SVGA_INDEX_PORT 0x0
-#define VMWARE_SVGA_VALUE_PORT 0x1
-
-//
-// Some of the device's register indices for basic framebuffer functionality.
-//
-typedef enum {
- VmwareSvgaRegId = 0,
- VmwareSvgaRegEnable = 1,
- VmwareSvgaRegWidth = 2,
- VmwareSvgaRegHeight = 3,
- VmwareSvgaRegMaxWidth = 4,
- VmwareSvgaRegMaxHeight = 5,
-
- VmwareSvgaRegBitsPerPixel = 7,
-
- VmwareSvgaRegRedMask = 9,
- VmwareSvgaRegGreenMask = 10,
- VmwareSvgaRegBlueMask = 11,
- VmwareSvgaRegBytesPerLine = 12,
-
- VmwareSvgaRegFbOffset = 14,
-
- VmwareSvgaRegFbSize = 16,
- VmwareSvgaRegCapabilities = 17,
-
- VmwareSvgaRegHostBitsPerPixel = 28,
-} VMWARE_SVGA_REGISTER;
-
-//
-// Values used with VmwareSvgaRegId for sanity-checking the device and getting
-// its version.
-//
-#define VMWARE_SVGA_MAGIC 0x900000U
-#define VMWARE_SVGA_MAKE_ID(ver) (VMWARE_SVGA_MAGIC << 8 | (ver))
-
-#define VMWARE_SVGA_VERSION_2 2
-#define VMWARE_SVGA_ID_2 VMWARE_SVGA_MAKE_ID (VMWARE_SVGA_VERSION_2)
-
-#define VMWARE_SVGA_VERSION_1 1
-#define VMWARE_SVGA_ID_1 VMWARE_SVGA_MAKE_ID (VMWARE_SVGA_VERSION_1)
-
-#define VMWARE_SVGA_VERSION_0 0
-#define VMWARE_SVGA_ID_0 VMWARE_SVGA_MAKE_ID (VMWARE_SVGA_VERSION_0)
-
-//
-// One of the capability bits advertised by VmwareSvgaRegCapabilities.
-//
-#define VMWARE_SVGA_CAP_8BIT_EMULATION BIT8
-
-#endif
--
2.18.0
^ permalink raw reply related [flat|nested] 23+ messages in thread* Re: [PATCH v2 4/5] Revert "OvmfPkg: VMWare SVGA display device register definitions"
2018-11-02 3:24 ` [PATCH v2 4/5] Revert "OvmfPkg: VMWare SVGA display device register definitions" yuchenlin
@ 2018-11-06 10:48 ` Laszlo Ersek
0 siblings, 0 replies; 23+ messages in thread
From: Laszlo Ersek @ 2018-11-06 10:48 UTC (permalink / raw)
To: yuchenlin, edk2-devel; +Cc: phil, jordan.l.justen, anthony.perard
On 11/02/18 04:24, yuchenlin via edk2-devel wrote:
> From: yuchenlin <yuchenlin@synology.com>
>
> This reverts commit 9bcca53fe466cdff397578328d9d87d257aba493.
>
> We reverted VMWare SVGA driver. We don't need these definitions too.
>
> Contributed-under: TianoCore Contribution Agreement 1.1
> Signed-off-by: yuchenlin <yuchenlin@synology.com>
> ---
> OvmfPkg/Include/IndustryStandard/VmwareSvga.h | 104 ------------------
> 1 file changed, 104 deletions(-)
> delete mode 100644 OvmfPkg/Include/IndustryStandard/VmwareSvga.h
Verified similarly to the last two patches in the series.
Reviewed-by: Laszlo Ersek <lersek@redhat.com>
^ permalink raw reply [flat|nested] 23+ messages in thread
* [PATCH v2 5/5] OvmfPkg: simply use the Bochs interface for vmsvga
2018-11-02 3:23 [PATCH v2 0/5] OvmfPkg: simply use the Bochs interface for vmsvga yuchenlin
` (3 preceding siblings ...)
2018-11-02 3:24 ` [PATCH v2 4/5] Revert "OvmfPkg: VMWare SVGA display device register definitions" yuchenlin
@ 2018-11-02 3:24 ` yuchenlin
2018-11-06 11:47 ` Laszlo Ersek
2018-11-05 22:52 ` [PATCH v2 0/5] " Laszlo Ersek
2018-11-12 14:16 ` Philippe Mathieu-Daudé
6 siblings, 1 reply; 23+ messages in thread
From: yuchenlin @ 2018-11-02 3:24 UTC (permalink / raw)
To: edk2-devel
Cc: jordan.l.justen, lersek, ard.biesheuvel, anthony.perard,
julien.grall, phil, kraxel, philmd, yuchenlin
From: yuchenlin <yuchenlin@synology.com>
BAR | std vga | vmsvga
---------------------------------
0 | Framebuffer | I/O space
1 | Reserved | Framebuffer
2 | MMIO | FIFO
Because of the PCI BARs difference between std vga and
vmsvga, we can not simply recognize the "QEMU VMWare SVGA"
as the QEMU_VIDEO_BOCHS_MMIO variant.
Instead, we remain variant QEMU_VIDEO_VMWARE_SVGA, and use
it for:
(1) Get framebuffer from correct PCI BAR
(2) Prevent using BAR2 for MMIO
Contributed-under: TianoCore Contribution Agreement 1.1
Signed-off-by: yuchenlin <yuchenlin@synology.com>
---
OvmfPkg/QemuVideoDxe/Driver.c | 18 ++++++++++++++++--
OvmfPkg/QemuVideoDxe/Gop.c | 3 ++-
OvmfPkg/QemuVideoDxe/Qemu.h | 2 ++
3 files changed, 20 insertions(+), 3 deletions(-)
diff --git a/OvmfPkg/QemuVideoDxe/Driver.c b/OvmfPkg/QemuVideoDxe/Driver.c
index 2304afd1e6..76d4a2d27e 100644
--- a/OvmfPkg/QemuVideoDxe/Driver.c
+++ b/OvmfPkg/QemuVideoDxe/Driver.c
@@ -69,6 +69,12 @@ QEMU_VIDEO_CARD gQemuVideoCardList[] = {
0x1050,
QEMU_VIDEO_BOCHS_MMIO,
L"QEMU VirtIO VGA"
+ },{
+ PCI_CLASS_DISPLAY_VGA,
+ 0x15ad,
+ 0x0405,
+ QEMU_VIDEO_VMWARE_SVGA,
+ L"QEMU VMWare SVGA"
},{
0 /* end of list */
}
@@ -256,6 +262,12 @@ QemuVideoControllerDriverStart (
goto ClosePciIo;
}
Private->Variant = Card->Variant;
+ if (Private->Variant == QEMU_VIDEO_VMWARE_SVGA) {
+ Private->FrameBufferVramBarIndex = PCI_BAR_IDX1;
+ } else {
+ Private->FrameBufferVramBarIndex = PCI_BAR_IDX0;
+ }
+
//
// IsQxl is based on the detected Card->Variant, which at a later point might
@@ -320,7 +332,8 @@ QemuVideoControllerDriverStart (
// Check if accessing the bochs interface works.
//
if (Private->Variant == QEMU_VIDEO_BOCHS_MMIO ||
- Private->Variant == QEMU_VIDEO_BOCHS) {
+ Private->Variant == QEMU_VIDEO_BOCHS ||
+ Private->Variant == QEMU_VIDEO_VMWARE_SVGA) {
UINT16 BochsId;
BochsId = BochsRead(Private, VBE_DISPI_INDEX_ID);
if ((BochsId & 0xFFF0) != VBE_DISPI_ID0) {
@@ -383,6 +396,7 @@ QemuVideoControllerDriverStart (
break;
case QEMU_VIDEO_BOCHS_MMIO:
case QEMU_VIDEO_BOCHS:
+ case QEMU_VIDEO_VMWARE_SVGA:
Status = QemuVideoBochsModeSetup (Private, IsQxl);
break;
default:
@@ -764,7 +778,7 @@ ClearScreen (
Private->PciIo->Mem.Write (
Private->PciIo,
EfiPciIoWidthFillUint32,
- 0,
+ Private->FrameBufferVramBarIndex,
0,
0x400000 >> 2,
&Color
diff --git a/OvmfPkg/QemuVideoDxe/Gop.c b/OvmfPkg/QemuVideoDxe/Gop.c
index d490fa7a2e..3abc5eeb36 100644
--- a/OvmfPkg/QemuVideoDxe/Gop.c
+++ b/OvmfPkg/QemuVideoDxe/Gop.c
@@ -60,7 +60,7 @@ QemuVideoCompleteModeData (
Private->PciIo->GetBarAttributes (
Private->PciIo,
- 0,
+ Private->FrameBufferVramBarIndex,
NULL,
(VOID**) &FrameBufDesc
);
@@ -177,6 +177,7 @@ Routine Description:
break;
case QEMU_VIDEO_BOCHS_MMIO:
case QEMU_VIDEO_BOCHS:
+ case QEMU_VIDEO_VMWARE_SVGA:
InitializeBochsGraphicsMode (Private, &QemuVideoBochsModes[ModeData->InternalModeIndex]);
break;
default:
diff --git a/OvmfPkg/QemuVideoDxe/Qemu.h b/OvmfPkg/QemuVideoDxe/Qemu.h
index d7da761705..3aac9eeca6 100644
--- a/OvmfPkg/QemuVideoDxe/Qemu.h
+++ b/OvmfPkg/QemuVideoDxe/Qemu.h
@@ -92,6 +92,7 @@ typedef enum {
QEMU_VIDEO_CIRRUS_5446,
QEMU_VIDEO_BOCHS,
QEMU_VIDEO_BOCHS_MMIO,
+ QEMU_VIDEO_VMWARE_SVGA,
} QEMU_VIDEO_VARIANT;
typedef struct {
@@ -120,6 +121,7 @@ typedef struct {
QEMU_VIDEO_VARIANT Variant;
FRAME_BUFFER_CONFIGURE *FrameBufferBltConfigure;
UINTN FrameBufferBltConfigureSize;
+ UINT8 FrameBufferVramBarIndex;
} QEMU_VIDEO_PRIVATE_DATA;
///
--
2.18.0
^ permalink raw reply related [flat|nested] 23+ messages in thread* Re: [PATCH v2 5/5] OvmfPkg: simply use the Bochs interface for vmsvga
2018-11-02 3:24 ` [PATCH v2 5/5] OvmfPkg: simply use the Bochs interface for vmsvga yuchenlin
@ 2018-11-06 11:47 ` Laszlo Ersek
2018-11-06 13:36 ` Laszlo Ersek
2018-11-07 2:36 ` yuchenlin
0 siblings, 2 replies; 23+ messages in thread
From: Laszlo Ersek @ 2018-11-06 11:47 UTC (permalink / raw)
To: yuchenlin
Cc: edk2-devel, phil, jordan.l.justen, anthony.perard,
Philippe Mathieu-Daudé, Gerd Hoffmann
I suggest the following:
On 11/02/18 04:24, yuchenlin via edk2-devel wrote:
> From: yuchenlin <yuchenlin@synology.com>
>
> BAR | std vga | vmsvga
> ---------------------------------
> 0 | Framebuffer | I/O space
> 1 | Reserved | Framebuffer
> 2 | MMIO | FIFO
>
> Because of the PCI BARs difference between std vga and
> vmsvga, we can not simply recognize the "QEMU VMWare SVGA"
> as the QEMU_VIDEO_BOCHS_MMIO variant.
>
> Instead, we remain variant QEMU_VIDEO_VMWARE_SVGA, and use
> it for:
>
> (1) Get framebuffer from correct PCI BAR
> (2) Prevent using BAR2 for MMIO
[a] The commit message should be udpated as follows:
- We cannot recognize VMW SVGA as BOCHS because that would confuse the
IsQxl setting in QemuVideoControllerDriverStart(),
- We cannot recognize VMW SVGA as BOCHS_MMIO because BAR2 on VMW SVGA is
not the BOCHS MMIO BAR (we can only use port IO).
Therefore the list of reasons for which we should introduce
QEMU_VIDEO_VMWARE_SVGA should name three reasons: both of the currently
listed reasons, plus "prevent mis-recognizing VMW SVGA as QXL" as the
third one.
>
> Contributed-under: TianoCore Contribution Agreement 1.1
> Signed-off-by: yuchenlin <yuchenlin@synology.com>
> ---
> OvmfPkg/QemuVideoDxe/Driver.c | 18 ++++++++++++++++--
> OvmfPkg/QemuVideoDxe/Gop.c | 3 ++-
> OvmfPkg/QemuVideoDxe/Qemu.h | 2 ++
> 3 files changed, 20 insertions(+), 3 deletions(-)
>
> diff --git a/OvmfPkg/QemuVideoDxe/Driver.c b/OvmfPkg/QemuVideoDxe/Driver.c
> index 2304afd1e6..76d4a2d27e 100644
> --- a/OvmfPkg/QemuVideoDxe/Driver.c
> +++ b/OvmfPkg/QemuVideoDxe/Driver.c
> @@ -69,6 +69,12 @@ QEMU_VIDEO_CARD gQemuVideoCardList[] = {
> 0x1050,
> QEMU_VIDEO_BOCHS_MMIO,
> L"QEMU VirtIO VGA"
> + },{
> + PCI_CLASS_DISPLAY_VGA,
> + 0x15ad,
> + 0x0405,
> + QEMU_VIDEO_VMWARE_SVGA,
> + L"QEMU VMWare SVGA"
> },{
> 0 /* end of list */
> }
> @@ -256,6 +262,12 @@ QemuVideoControllerDriverStart (
> goto ClosePciIo;
> }
> Private->Variant = Card->Variant;
> + if (Private->Variant == QEMU_VIDEO_VMWARE_SVGA) {
> + Private->FrameBufferVramBarIndex = PCI_BAR_IDX1;
> + } else {
> + Private->FrameBufferVramBarIndex = PCI_BAR_IDX0;
> + }
> +
>
> //
> // IsQxl is based on the detected Card->Variant, which at a later point might
> @@ -320,7 +332,8 @@ QemuVideoControllerDriverStart (
> // Check if accessing the bochs interface works.
> //
> if (Private->Variant == QEMU_VIDEO_BOCHS_MMIO ||
> - Private->Variant == QEMU_VIDEO_BOCHS) {
> + Private->Variant == QEMU_VIDEO_BOCHS ||
> + Private->Variant == QEMU_VIDEO_VMWARE_SVGA) {
> UINT16 BochsId;
> BochsId = BochsRead(Private, VBE_DISPI_INDEX_ID);
> if ((BochsId & 0xFFF0) != VBE_DISPI_ID0) {
> @@ -383,6 +396,7 @@ QemuVideoControllerDriverStart (
> break;
> case QEMU_VIDEO_BOCHS_MMIO:
> case QEMU_VIDEO_BOCHS:
> + case QEMU_VIDEO_VMWARE_SVGA:
> Status = QemuVideoBochsModeSetup (Private, IsQxl);
> break;
> default:
> @@ -764,7 +778,7 @@ ClearScreen (
> Private->PciIo->Mem.Write (
> Private->PciIo,
> EfiPciIoWidthFillUint32,
> - 0,
> + Private->FrameBufferVramBarIndex,
> 0,
> 0x400000 >> 2,
> &Color
> diff --git a/OvmfPkg/QemuVideoDxe/Gop.c b/OvmfPkg/QemuVideoDxe/Gop.c
> index d490fa7a2e..3abc5eeb36 100644
> --- a/OvmfPkg/QemuVideoDxe/Gop.c
> +++ b/OvmfPkg/QemuVideoDxe/Gop.c
> @@ -60,7 +60,7 @@ QemuVideoCompleteModeData (
>
> Private->PciIo->GetBarAttributes (
> Private->PciIo,
> - 0,
> + Private->FrameBufferVramBarIndex,
> NULL,
> (VOID**) &FrameBufDesc
> );
> @@ -177,6 +177,7 @@ Routine Description:
> break;
> case QEMU_VIDEO_BOCHS_MMIO:
> case QEMU_VIDEO_BOCHS:
> + case QEMU_VIDEO_VMWARE_SVGA:
> InitializeBochsGraphicsMode (Private, &QemuVideoBochsModes[ModeData->InternalModeIndex]);
> break;
> default:
> diff --git a/OvmfPkg/QemuVideoDxe/Qemu.h b/OvmfPkg/QemuVideoDxe/Qemu.h
> index d7da761705..3aac9eeca6 100644
> --- a/OvmfPkg/QemuVideoDxe/Qemu.h
> +++ b/OvmfPkg/QemuVideoDxe/Qemu.h
> @@ -92,6 +92,7 @@ typedef enum {
> QEMU_VIDEO_CIRRUS_5446,
> QEMU_VIDEO_BOCHS,
> QEMU_VIDEO_BOCHS_MMIO,
> + QEMU_VIDEO_VMWARE_SVGA,
> } QEMU_VIDEO_VARIANT;
>
> typedef struct {
> @@ -120,6 +121,7 @@ typedef struct {
> QEMU_VIDEO_VARIANT Variant;
> FRAME_BUFFER_CONFIGURE *FrameBufferBltConfigure;
> UINTN FrameBufferBltConfigureSize;
> + UINT8 FrameBufferVramBarIndex;
> } QEMU_VIDEO_PRIVATE_DATA;
>
> ///
>
[b] How about the following -- incremental, to be squashed -- patch:
> diff --git a/OvmfPkg/QemuVideoDxe/Driver.c b/OvmfPkg/QemuVideoDxe/Driver.c
> index 76d4a2d27e7e..8e02700d3960 100644
> --- a/OvmfPkg/QemuVideoDxe/Driver.c
> +++ b/OvmfPkg/QemuVideoDxe/Driver.c
> @@ -262,12 +262,6 @@ QemuVideoControllerDriverStart (
> goto ClosePciIo;
> }
> Private->Variant = Card->Variant;
> - if (Private->Variant == QEMU_VIDEO_VMWARE_SVGA) {
> - Private->FrameBufferVramBarIndex = PCI_BAR_IDX1;
> - } else {
> - Private->FrameBufferVramBarIndex = PCI_BAR_IDX0;
> - }
> -
>
> //
> // IsQxl is based on the detected Card->Variant, which at a later point
> might
> @@ -328,12 +322,19 @@ QemuVideoControllerDriverStart (
> }
> }
>
> + //
> + // VMWare SVGA is handled like Bochs (with port IO only).
> + //
> + if (Private->Variant == QEMU_VIDEO_VMWARE_SVGA) {
> + Private->Variant = QEMU_VIDEO_BOCHS;
> + Private->FrameBufferVramBarIndex = PCI_BAR_IDX1;
> + }
> +
> //
> // Check if accessing the bochs interface works.
> //
> if (Private->Variant == QEMU_VIDEO_BOCHS_MMIO ||
> - Private->Variant == QEMU_VIDEO_BOCHS ||
> - Private->Variant == QEMU_VIDEO_VMWARE_SVGA) {
> + Private->Variant == QEMU_VIDEO_BOCHS) {
> UINT16 BochsId;
> BochsId = BochsRead(Private, VBE_DISPI_INDEX_ID);
> if ((BochsId & 0xFFF0) != VBE_DISPI_ID0) {
> @@ -396,7 +397,6 @@ QemuVideoControllerDriverStart (
> break;
> case QEMU_VIDEO_BOCHS_MMIO:
> case QEMU_VIDEO_BOCHS:
> - case QEMU_VIDEO_VMWARE_SVGA:
> Status = QemuVideoBochsModeSetup (Private, IsQxl);
> break;
> default:
> diff --git a/OvmfPkg/QemuVideoDxe/Gop.c b/OvmfPkg/QemuVideoDxe/Gop.c
> index 3abc5eeb36a6..6f542d9eac05 100644
> --- a/OvmfPkg/QemuVideoDxe/Gop.c
> +++ b/OvmfPkg/QemuVideoDxe/Gop.c
> @@ -177,7 +177,6 @@ Routine Description:
> break;
> case QEMU_VIDEO_BOCHS_MMIO:
> case QEMU_VIDEO_BOCHS:
> - case QEMU_VIDEO_VMWARE_SVGA:
> InitializeBochsGraphicsMode (Private,
> &QemuVideoBochsModes[ModeData->InternalModeIndex]);
> break;
> default:
This would produce the following -- squashed -- patch, for v3:
> diff --git a/OvmfPkg/QemuVideoDxe/Qemu.h b/OvmfPkg/QemuVideoDxe/Qemu.h
> index d7da761705a1..3aac9eeca687 100644
> --- a/OvmfPkg/QemuVideoDxe/Qemu.h
> +++ b/OvmfPkg/QemuVideoDxe/Qemu.h
> @@ -92,6 +92,7 @@ typedef enum {
> QEMU_VIDEO_CIRRUS_5446,
> QEMU_VIDEO_BOCHS,
> QEMU_VIDEO_BOCHS_MMIO,
> + QEMU_VIDEO_VMWARE_SVGA,
> } QEMU_VIDEO_VARIANT;
>
> typedef struct {
> @@ -120,6 +121,7 @@ typedef struct {
> QEMU_VIDEO_VARIANT Variant;
> FRAME_BUFFER_CONFIGURE *FrameBufferBltConfigure;
> UINTN FrameBufferBltConfigureSize;
> + UINT8 FrameBufferVramBarIndex;
> } QEMU_VIDEO_PRIVATE_DATA;
>
> ///
> diff --git a/OvmfPkg/QemuVideoDxe/Driver.c b/OvmfPkg/QemuVideoDxe/Driver.c
> index 2304afd1e686..8e02700d3960 100644
> --- a/OvmfPkg/QemuVideoDxe/Driver.c
> +++ b/OvmfPkg/QemuVideoDxe/Driver.c
> @@ -69,6 +69,12 @@ QEMU_VIDEO_CARD gQemuVideoCardList[] = {
> 0x1050,
> QEMU_VIDEO_BOCHS_MMIO,
> L"QEMU VirtIO VGA"
> + },{
> + PCI_CLASS_DISPLAY_VGA,
> + 0x15ad,
> + 0x0405,
> + QEMU_VIDEO_VMWARE_SVGA,
> + L"QEMU VMWare SVGA"
> },{
> 0 /* end of list */
> }
> @@ -316,6 +322,14 @@ QemuVideoControllerDriverStart (
> }
> }
>
> + //
> + // VMWare SVGA is handled like Bochs (with port IO only).
> + //
> + if (Private->Variant == QEMU_VIDEO_VMWARE_SVGA) {
> + Private->Variant = QEMU_VIDEO_BOCHS;
> + Private->FrameBufferVramBarIndex = PCI_BAR_IDX1;
> + }
> +
> //
> // Check if accessing the bochs interface works.
> //
> @@ -764,7 +778,7 @@ ClearScreen (
> Private->PciIo->Mem.Write (
> Private->PciIo,
> EfiPciIoWidthFillUint32,
> - 0,
> + Private->FrameBufferVramBarIndex,
> 0,
> 0x400000 >> 2,
> &Color
> diff --git a/OvmfPkg/QemuVideoDxe/Gop.c b/OvmfPkg/QemuVideoDxe/Gop.c
> index d490fa7a2e6f..6f542d9eac05 100644
> --- a/OvmfPkg/QemuVideoDxe/Gop.c
> +++ b/OvmfPkg/QemuVideoDxe/Gop.c
> @@ -60,7 +60,7 @@ QemuVideoCompleteModeData (
>
> Private->PciIo->GetBarAttributes (
> Private->PciIo,
> - 0,
> + Private->FrameBufferVramBarIndex,
> NULL,
> (VOID**) &FrameBufDesc
> );
For me this is much easier to understand.
... While we discuss this, I'll go ahead and push the first four
patches. The code being reverted is dead anyway. I'll report back about
the commit hashes.
Thanks,
Laszlo
^ permalink raw reply [flat|nested] 23+ messages in thread* Re: [PATCH v2 5/5] OvmfPkg: simply use the Bochs interface for vmsvga
2018-11-06 11:47 ` Laszlo Ersek
@ 2018-11-06 13:36 ` Laszlo Ersek
2018-11-06 13:44 ` Philippe Mathieu-Daudé
2018-11-07 2:36 ` yuchenlin
1 sibling, 1 reply; 23+ messages in thread
From: Laszlo Ersek @ 2018-11-06 13:36 UTC (permalink / raw)
To: yuchenlin
Cc: edk2-devel, phil, jordan.l.justen, anthony.perard,
Philippe Mathieu-Daudé, Gerd Hoffmann
On 11/06/18 12:47, Laszlo Ersek wrote:
> ... While we discuss this, I'll go ahead and push the first four
> patches. The code being reverted is dead anyway. I'll report back about
> the commit hashes.
Before pushing the first four patches, I regression-tested them as well.
Using: Cirrus, stdvga, and QXL. My QEMU version was
v3.0.0-1763-gb2f7a038bb4c. The machine type was "pc-q35-3.0".
For the first four patches:
- Regression-tested-by: Laszlo Ersek <lersek@redhat.com>,
- pushed them as commit range 62ea70e31285..328409ce8de7.
Thanks
Laszlo
^ permalink raw reply [flat|nested] 23+ messages in thread
* Re: [PATCH v2 5/5] OvmfPkg: simply use the Bochs interface for vmsvga
2018-11-06 13:36 ` Laszlo Ersek
@ 2018-11-06 13:44 ` Philippe Mathieu-Daudé
2018-11-06 16:20 ` Laszlo Ersek
` (2 more replies)
0 siblings, 3 replies; 23+ messages in thread
From: Philippe Mathieu-Daudé @ 2018-11-06 13:44 UTC (permalink / raw)
To: Laszlo Ersek, yuchenlin
Cc: edk2-devel, phil, jordan.l.justen, anthony.perard, Gerd Hoffmann
On 6/11/18 14:36, Laszlo Ersek wrote:
> On 11/06/18 12:47, Laszlo Ersek wrote:
>
>> ... While we discuss this, I'll go ahead and push the first four
>> patches. The code being reverted is dead anyway. I'll report back about
>> the commit hashes.
>
> Before pushing the first four patches, I regression-tested them as well.
> Using: Cirrus, stdvga, and QXL. My QEMU version was
> v3.0.0-1763-gb2f7a038bb4c. The machine type was "pc-q35-3.0".
>
> For the first four patches:
> - Regression-tested-by: Laszlo Ersek <lersek@redhat.com>,
> - pushed them as commit range 62ea70e31285..328409ce8de7.
Thanks Laszlo!
A bit late, but 1-4 reviewed too.
>
> Thanks
> Laszlo
>
^ permalink raw reply [flat|nested] 23+ messages in thread* Re: [PATCH v2 5/5] OvmfPkg: simply use the Bochs interface for vmsvga
2018-11-06 13:44 ` Philippe Mathieu-Daudé
@ 2018-11-06 16:20 ` Laszlo Ersek
2018-11-07 2:37 ` yuchenlin
2018-11-12 11:19 ` Laszlo Ersek
2 siblings, 0 replies; 23+ messages in thread
From: Laszlo Ersek @ 2018-11-06 16:20 UTC (permalink / raw)
To: Philippe Mathieu-Daudé, yuchenlin
Cc: edk2-devel, phil, jordan.l.justen, anthony.perard, Gerd Hoffmann
On 11/06/18 14:44, Philippe Mathieu-Daudé wrote:
> On 6/11/18 14:36, Laszlo Ersek wrote:
>> On 11/06/18 12:47, Laszlo Ersek wrote:
>>
>>> ... While we discuss this, I'll go ahead and push the first four
>>> patches. The code being reverted is dead anyway. I'll report back about
>>> the commit hashes.
>>
>> Before pushing the first four patches, I regression-tested them as well.
>> Using: Cirrus, stdvga, and QXL. My QEMU version was
>> v3.0.0-1763-gb2f7a038bb4c. The machine type was "pc-q35-3.0".
>>
>> For the first four patches:
>> - Regression-tested-by: Laszlo Ersek <lersek@redhat.com>,
>> - pushed them as commit range 62ea70e31285..328409ce8de7.
>
> Thanks Laszlo!
> A bit late, but 1-4 reviewed too.
I apologize, I should have waited longer.
Laszlo
^ permalink raw reply [flat|nested] 23+ messages in thread
* Re: [PATCH v2 5/5] OvmfPkg: simply use the Bochs interface for vmsvga
2018-11-06 13:44 ` Philippe Mathieu-Daudé
2018-11-06 16:20 ` Laszlo Ersek
@ 2018-11-07 2:37 ` yuchenlin
2018-11-12 11:19 ` Laszlo Ersek
2 siblings, 0 replies; 23+ messages in thread
From: yuchenlin @ 2018-11-07 2:37 UTC (permalink / raw)
To: Philippe Mathieu-Daudé
Cc: Laszlo Ersek, edk2-devel, phil, jordan.l.justen, anthony.perard,
Gerd Hoffmann
On 2018-11-06 21:44, Philippe Mathieu-Daudé wrote:
> On 6/11/18 14:36, Laszlo Ersek wrote:
>> On 11/06/18 12:47, Laszlo Ersek wrote:
>>
>>> ... While we discuss this, I'll go ahead and push the first four
>>> patches. The code being reverted is dead anyway. I'll report back
>>> about
>>> the commit hashes.
>>
>> Before pushing the first four patches, I regression-tested them as
>> well.
>> Using: Cirrus, stdvga, and QXL. My QEMU version was
>> v3.0.0-1763-gb2f7a038bb4c. The machine type was "pc-q35-3.0".
>>
>> For the first four patches:
>> - Regression-tested-by: Laszlo Ersek <lersek@redhat.com>,
>> - pushed them as commit range 62ea70e31285..328409ce8de7.
>
> Thanks Laszlo!
> A bit late, but 1-4 reviewed too.
>
>>
>> Thanks
>> Laszlo
>>
Thank you for your effort on this series!
Thanks,
yuchenlin
^ permalink raw reply [flat|nested] 23+ messages in thread
* Re: [PATCH v2 5/5] OvmfPkg: simply use the Bochs interface for vmsvga
2018-11-06 13:44 ` Philippe Mathieu-Daudé
2018-11-06 16:20 ` Laszlo Ersek
2018-11-07 2:37 ` yuchenlin
@ 2018-11-12 11:19 ` Laszlo Ersek
2018-11-12 11:28 ` Philippe Mathieu-Daudé
2 siblings, 1 reply; 23+ messages in thread
From: Laszlo Ersek @ 2018-11-12 11:19 UTC (permalink / raw)
To: Philippe Mathieu-Daudé, yuchenlin
Cc: anthony.perard, jordan.l.justen, edk2-devel, phil
Hi Phil,
On 11/06/18 14:44, Philippe Mathieu-Daudé wrote:
> On 6/11/18 14:36, Laszlo Ersek wrote:
>> On 11/06/18 12:47, Laszlo Ersek wrote:
>>
>>> ... While we discuss this, I'll go ahead and push the first four
>>> patches. The code being reverted is dead anyway. I'll report back about
>>> the commit hashes.
>>
>> Before pushing the first four patches, I regression-tested them as well.
>> Using: Cirrus, stdvga, and QXL. My QEMU version was
>> v3.0.0-1763-gb2f7a038bb4c. The machine type was "pc-q35-3.0".
>>
>> For the first four patches:
>> - Regression-tested-by: Laszlo Ersek <lersek@redhat.com>,
>> - pushed them as commit range 62ea70e31285..328409ce8de7.
>
> Thanks Laszlo!
> A bit late, but 1-4 reviewed too.
can you please formally state your R-b, for the first four patches? For
when I apply them again, after edk2-stable201811.
Thanks!
Laszlo
^ permalink raw reply [flat|nested] 23+ messages in thread
* Re: [PATCH v2 5/5] OvmfPkg: simply use the Bochs interface for vmsvga
2018-11-12 11:19 ` Laszlo Ersek
@ 2018-11-12 11:28 ` Philippe Mathieu-Daudé
2018-11-12 12:21 ` Laszlo Ersek
0 siblings, 1 reply; 23+ messages in thread
From: Philippe Mathieu-Daudé @ 2018-11-12 11:28 UTC (permalink / raw)
To: Laszlo Ersek
Cc: yuchenlin, Anthony Perard, Jordan Justen, edk2-devel-01,
Phil Dennis-Jordan
On Mon, Nov 12, 2018 at 12:20 PM Laszlo Ersek <lersek@redhat.com> wrote:
> On 11/06/18 14:44, Philippe Mathieu-Daudé wrote:
> > On 6/11/18 14:36, Laszlo Ersek wrote:
> >> On 11/06/18 12:47, Laszlo Ersek wrote:
> >>
> >>> ... While we discuss this, I'll go ahead and push the first four
> >>> patches. The code being reverted is dead anyway. I'll report back about
> >>> the commit hashes.
> >>
> >> Before pushing the first four patches, I regression-tested them as well.
> >> Using: Cirrus, stdvga, and QXL. My QEMU version was
> >> v3.0.0-1763-gb2f7a038bb4c. The machine type was "pc-q35-3.0".
> >>
> >> For the first four patches:
> >> - Regression-tested-by: Laszlo Ersek <lersek@redhat.com>,
> >> - pushed them as commit range 62ea70e31285..328409ce8de7.
> >
> > Thanks Laszlo!
> > A bit late, but 1-4 reviewed too.
>
> can you please formally state your R-b, for the first four patches? For
> when I apply them again, after edk2-stable201811.
Sure, I was not aware the "reply to cover" option was not formal for
this list, I'll do.
(TIL: "reply to cover" is list CoC specific)
^ permalink raw reply [flat|nested] 23+ messages in thread
* Re: [PATCH v2 5/5] OvmfPkg: simply use the Bochs interface for vmsvga
2018-11-12 11:28 ` Philippe Mathieu-Daudé
@ 2018-11-12 12:21 ` Laszlo Ersek
0 siblings, 0 replies; 23+ messages in thread
From: Laszlo Ersek @ 2018-11-12 12:21 UTC (permalink / raw)
To: Philippe Mathieu-Daudé
Cc: yuchenlin, Anthony Perard, Jordan Justen, edk2-devel-01,
Phil Dennis-Jordan
On 11/12/18 12:28, Philippe Mathieu-Daudé wrote:
> On Mon, Nov 12, 2018 at 12:20 PM Laszlo Ersek <lersek@redhat.com>
> wrote:
>> On 11/06/18 14:44, Philippe Mathieu-Daudé wrote:
>>> On 6/11/18 14:36, Laszlo Ersek wrote:
>>>> On 11/06/18 12:47, Laszlo Ersek wrote:
>>>>
>>>>> ... While we discuss this, I'll go ahead and push the first four
>>>>> patches. The code being reverted is dead anyway. I'll report back
>>>>> about the commit hashes.
>>>>
>>>> Before pushing the first four patches, I regression-tested them as
>>>> well.
>>>> Using: Cirrus, stdvga, and QXL. My QEMU version was
>>>> v3.0.0-1763-gb2f7a038bb4c. The machine type was "pc-q35-3.0".
>>>>
>>>> For the first four patches:
>>>> - Regression-tested-by: Laszlo Ersek <lersek@redhat.com>,
>>>> - pushed them as commit range 62ea70e31285..328409ce8de7.
>>>
>>> Thanks Laszlo!
>>> A bit late, but 1-4 reviewed too.
>>
>> can you please formally state your R-b, for the first four patches?
>> For when I apply them again, after edk2-stable201811.
>
> Sure, I was not aware the "reply to cover" option was not formal for
> this list, I'll do.
> (TIL: "reply to cover" is list CoC specific)
"reply to cover letter" is perfectly fine on this list as well (it never
occurred to me that it would be unacceptable for any list in the first
place) -- but where did you respond to:
[edk2] [PATCH v2 0/5] OvmfPkg: simply use the Bochs interface for vmsvga
http://mid.mail-archive.com/20181102032402.19686-1-yuchenlin@synology.com
?
... Ahhh, wait, now I see where our misunderstanding is. :) By
"formally", I did not mean that you should respond to every single patch
(#1 through #4) separately, with your R-b. Responding under the cover
letter, or even just in this thread, is perfectly fine. What I'm asking
for instead is that you please "formally" spell out your Reviewed-by
tag. In edk2 we strongly prefer carrying R-b tags from the mailing list
to commit messages verbatim (i.e. with the clipboard). We dislike
synthesizing formal R-b tags from messages like "sure, reviewed by me
too".
So, again, in this context, "formally" only means that you please repeat
your "reviewed too" message "formally" with an R-b tag, spelled out. A
response in this thread is perfectly fine.
Sorry about the confusion!
Laszlo
^ permalink raw reply [flat|nested] 23+ messages in thread
* Re: [PATCH v2 5/5] OvmfPkg: simply use the Bochs interface for vmsvga
2018-11-06 11:47 ` Laszlo Ersek
2018-11-06 13:36 ` Laszlo Ersek
@ 2018-11-07 2:36 ` yuchenlin
1 sibling, 0 replies; 23+ messages in thread
From: yuchenlin @ 2018-11-07 2:36 UTC (permalink / raw)
To: Laszlo Ersek
Cc: edk2-devel, phil, jordan.l.justen, anthony.perard,
Philippe Mathieu-Daudé, Gerd Hoffmann
On 2018-11-06 19:47, Laszlo Ersek wrote:
> I suggest the following:
>
> On 11/02/18 04:24, yuchenlin via edk2-devel wrote:
>> From: yuchenlin <yuchenlin@synology.com>
>>
>> BAR | std vga | vmsvga
>> ---------------------------------
>> 0 | Framebuffer | I/O space
>> 1 | Reserved | Framebuffer
>> 2 | MMIO | FIFO
>>
>> Because of the PCI BARs difference between std vga and
>> vmsvga, we can not simply recognize the "QEMU VMWare SVGA"
>> as the QEMU_VIDEO_BOCHS_MMIO variant.
>>
>> Instead, we remain variant QEMU_VIDEO_VMWARE_SVGA, and use
>> it for:
>>
>> (1) Get framebuffer from correct PCI BAR
>> (2) Prevent using BAR2 for MMIO
>
> [a] The commit message should be udpated as follows:
>
> - We cannot recognize VMW SVGA as BOCHS because that would confuse the
> IsQxl setting in QemuVideoControllerDriverStart(),
>
> - We cannot recognize VMW SVGA as BOCHS_MMIO because BAR2 on VMW SVGA
> is
> not the BOCHS MMIO BAR (we can only use port IO).
>
> Therefore the list of reasons for which we should introduce
> QEMU_VIDEO_VMWARE_SVGA should name three reasons: both of the currently
> listed reasons, plus "prevent mis-recognizing VMW SVGA as QXL" as the
> third one.
>
Will do.
Thanks,
yuchenlin
>>
>> Contributed-under: TianoCore Contribution Agreement 1.1
>> Signed-off-by: yuchenlin <yuchenlin@synology.com>
>> ---
>> OvmfPkg/QemuVideoDxe/Driver.c | 18 ++++++++++++++++--
>> OvmfPkg/QemuVideoDxe/Gop.c | 3 ++-
>> OvmfPkg/QemuVideoDxe/Qemu.h | 2 ++
>> 3 files changed, 20 insertions(+), 3 deletions(-)
>>
>> diff --git a/OvmfPkg/QemuVideoDxe/Driver.c
>> b/OvmfPkg/QemuVideoDxe/Driver.c
>> index 2304afd1e6..76d4a2d27e 100644
>> --- a/OvmfPkg/QemuVideoDxe/Driver.c
>> +++ b/OvmfPkg/QemuVideoDxe/Driver.c
>> @@ -69,6 +69,12 @@ QEMU_VIDEO_CARD gQemuVideoCardList[] = {
>> 0x1050,
>> QEMU_VIDEO_BOCHS_MMIO,
>> L"QEMU VirtIO VGA"
>> + },{
>> + PCI_CLASS_DISPLAY_VGA,
>> + 0x15ad,
>> + 0x0405,
>> + QEMU_VIDEO_VMWARE_SVGA,
>> + L"QEMU VMWare SVGA"
>> },{
>> 0 /* end of list */
>> }
>> @@ -256,6 +262,12 @@ QemuVideoControllerDriverStart (
>> goto ClosePciIo;
>> }
>> Private->Variant = Card->Variant;
>> + if (Private->Variant == QEMU_VIDEO_VMWARE_SVGA) {
>> + Private->FrameBufferVramBarIndex = PCI_BAR_IDX1;
>> + } else {
>> + Private->FrameBufferVramBarIndex = PCI_BAR_IDX0;
>> + }
>> +
>>
>> //
>> // IsQxl is based on the detected Card->Variant, which at a later
>> point might
>> @@ -320,7 +332,8 @@ QemuVideoControllerDriverStart (
>> // Check if accessing the bochs interface works.
>> //
>> if (Private->Variant == QEMU_VIDEO_BOCHS_MMIO ||
>> - Private->Variant == QEMU_VIDEO_BOCHS) {
>> + Private->Variant == QEMU_VIDEO_BOCHS ||
>> + Private->Variant == QEMU_VIDEO_VMWARE_SVGA) {
>> UINT16 BochsId;
>> BochsId = BochsRead(Private, VBE_DISPI_INDEX_ID);
>> if ((BochsId & 0xFFF0) != VBE_DISPI_ID0) {
>> @@ -383,6 +396,7 @@ QemuVideoControllerDriverStart (
>> break;
>> case QEMU_VIDEO_BOCHS_MMIO:
>> case QEMU_VIDEO_BOCHS:
>> + case QEMU_VIDEO_VMWARE_SVGA:
>> Status = QemuVideoBochsModeSetup (Private, IsQxl);
>> break;
>> default:
>> @@ -764,7 +778,7 @@ ClearScreen (
>> Private->PciIo->Mem.Write (
>> Private->PciIo,
>> EfiPciIoWidthFillUint32,
>> - 0,
>> + Private->FrameBufferVramBarIndex,
>> 0,
>> 0x400000 >> 2,
>> &Color
>> diff --git a/OvmfPkg/QemuVideoDxe/Gop.c b/OvmfPkg/QemuVideoDxe/Gop.c
>> index d490fa7a2e..3abc5eeb36 100644
>> --- a/OvmfPkg/QemuVideoDxe/Gop.c
>> +++ b/OvmfPkg/QemuVideoDxe/Gop.c
>> @@ -60,7 +60,7 @@ QemuVideoCompleteModeData (
>>
>> Private->PciIo->GetBarAttributes (
>> Private->PciIo,
>> - 0,
>> + Private->FrameBufferVramBarIndex,
>> NULL,
>> (VOID**) &FrameBufDesc
>> );
>> @@ -177,6 +177,7 @@ Routine Description:
>> break;
>> case QEMU_VIDEO_BOCHS_MMIO:
>> case QEMU_VIDEO_BOCHS:
>> + case QEMU_VIDEO_VMWARE_SVGA:
>> InitializeBochsGraphicsMode (Private,
>> &QemuVideoBochsModes[ModeData->InternalModeIndex]);
>> break;
>> default:
>> diff --git a/OvmfPkg/QemuVideoDxe/Qemu.h b/OvmfPkg/QemuVideoDxe/Qemu.h
>> index d7da761705..3aac9eeca6 100644
>> --- a/OvmfPkg/QemuVideoDxe/Qemu.h
>> +++ b/OvmfPkg/QemuVideoDxe/Qemu.h
>> @@ -92,6 +92,7 @@ typedef enum {
>> QEMU_VIDEO_CIRRUS_5446,
>> QEMU_VIDEO_BOCHS,
>> QEMU_VIDEO_BOCHS_MMIO,
>> + QEMU_VIDEO_VMWARE_SVGA,
>> } QEMU_VIDEO_VARIANT;
>>
>> typedef struct {
>> @@ -120,6 +121,7 @@ typedef struct {
>> QEMU_VIDEO_VARIANT Variant;
>> FRAME_BUFFER_CONFIGURE *FrameBufferBltConfigure;
>> UINTN FrameBufferBltConfigureSize;
>> + UINT8 FrameBufferVramBarIndex;
>> } QEMU_VIDEO_PRIVATE_DATA;
>>
>> ///
>>
>
> [b] How about the following -- incremental, to be squashed -- patch:
>
>> diff --git a/OvmfPkg/QemuVideoDxe/Driver.c
>> b/OvmfPkg/QemuVideoDxe/Driver.c
>> index 76d4a2d27e7e..8e02700d3960 100644
>> --- a/OvmfPkg/QemuVideoDxe/Driver.c
>> +++ b/OvmfPkg/QemuVideoDxe/Driver.c
>> @@ -262,12 +262,6 @@ QemuVideoControllerDriverStart (
>> goto ClosePciIo;
>> }
>> Private->Variant = Card->Variant;
>> - if (Private->Variant == QEMU_VIDEO_VMWARE_SVGA) {
>> - Private->FrameBufferVramBarIndex = PCI_BAR_IDX1;
>> - } else {
>> - Private->FrameBufferVramBarIndex = PCI_BAR_IDX0;
>> - }
>> -
>>
>> //
>> // IsQxl is based on the detected Card->Variant, which at a later
>> point
>> might
>> @@ -328,12 +322,19 @@ QemuVideoControllerDriverStart (
>> }
>> }
>>
>> + //
>> + // VMWare SVGA is handled like Bochs (with port IO only).
>> + //
>> + if (Private->Variant == QEMU_VIDEO_VMWARE_SVGA) {
>> + Private->Variant = QEMU_VIDEO_BOCHS;
>> + Private->FrameBufferVramBarIndex = PCI_BAR_IDX1;
>> + }
>> +
>> //
>> // Check if accessing the bochs interface works.
>> //
>> if (Private->Variant == QEMU_VIDEO_BOCHS_MMIO ||
>> - Private->Variant == QEMU_VIDEO_BOCHS ||
>> - Private->Variant == QEMU_VIDEO_VMWARE_SVGA) {
>> + Private->Variant == QEMU_VIDEO_BOCHS) {
>> UINT16 BochsId;
>> BochsId = BochsRead(Private, VBE_DISPI_INDEX_ID);
>> if ((BochsId & 0xFFF0) != VBE_DISPI_ID0) {
>> @@ -396,7 +397,6 @@ QemuVideoControllerDriverStart (
>> break;
>> case QEMU_VIDEO_BOCHS_MMIO:
>> case QEMU_VIDEO_BOCHS:
>> - case QEMU_VIDEO_VMWARE_SVGA:
>> Status = QemuVideoBochsModeSetup (Private, IsQxl);
>> break;
>> default:
>> diff --git a/OvmfPkg/QemuVideoDxe/Gop.c b/OvmfPkg/QemuVideoDxe/Gop.c
>> index 3abc5eeb36a6..6f542d9eac05 100644
>> --- a/OvmfPkg/QemuVideoDxe/Gop.c
>> +++ b/OvmfPkg/QemuVideoDxe/Gop.c
>> @@ -177,7 +177,6 @@ Routine Description:
>> break;
>> case QEMU_VIDEO_BOCHS_MMIO:
>> case QEMU_VIDEO_BOCHS:
>> - case QEMU_VIDEO_VMWARE_SVGA:
>> InitializeBochsGraphicsMode (Private,
>> &QemuVideoBochsModes[ModeData->InternalModeIndex]);
>> break;
>> default:
>
> This would produce the following -- squashed -- patch, for v3:
>
>> diff --git a/OvmfPkg/QemuVideoDxe/Qemu.h b/OvmfPkg/QemuVideoDxe/Qemu.h
>> index d7da761705a1..3aac9eeca687 100644
>> --- a/OvmfPkg/QemuVideoDxe/Qemu.h
>> +++ b/OvmfPkg/QemuVideoDxe/Qemu.h
>> @@ -92,6 +92,7 @@ typedef enum {
>> QEMU_VIDEO_CIRRUS_5446,
>> QEMU_VIDEO_BOCHS,
>> QEMU_VIDEO_BOCHS_MMIO,
>> + QEMU_VIDEO_VMWARE_SVGA,
>> } QEMU_VIDEO_VARIANT;
>>
>> typedef struct {
>> @@ -120,6 +121,7 @@ typedef struct {
>> QEMU_VIDEO_VARIANT Variant;
>> FRAME_BUFFER_CONFIGURE *FrameBufferBltConfigure;
>> UINTN FrameBufferBltConfigureSize;
>> + UINT8 FrameBufferVramBarIndex;
>> } QEMU_VIDEO_PRIVATE_DATA;
>>
>> ///
>> diff --git a/OvmfPkg/QemuVideoDxe/Driver.c
>> b/OvmfPkg/QemuVideoDxe/Driver.c
>> index 2304afd1e686..8e02700d3960 100644
>> --- a/OvmfPkg/QemuVideoDxe/Driver.c
>> +++ b/OvmfPkg/QemuVideoDxe/Driver.c
>> @@ -69,6 +69,12 @@ QEMU_VIDEO_CARD gQemuVideoCardList[] = {
>> 0x1050,
>> QEMU_VIDEO_BOCHS_MMIO,
>> L"QEMU VirtIO VGA"
>> + },{
>> + PCI_CLASS_DISPLAY_VGA,
>> + 0x15ad,
>> + 0x0405,
>> + QEMU_VIDEO_VMWARE_SVGA,
>> + L"QEMU VMWare SVGA"
>> },{
>> 0 /* end of list */
>> }
>> @@ -316,6 +322,14 @@ QemuVideoControllerDriverStart (
>> }
>> }
>>
>> + //
>> + // VMWare SVGA is handled like Bochs (with port IO only).
>> + //
>> + if (Private->Variant == QEMU_VIDEO_VMWARE_SVGA) {
>> + Private->Variant = QEMU_VIDEO_BOCHS;
>> + Private->FrameBufferVramBarIndex = PCI_BAR_IDX1;
>> + }
>> +
>> //
>> // Check if accessing the bochs interface works.
>> //
>> @@ -764,7 +778,7 @@ ClearScreen (
>> Private->PciIo->Mem.Write (
>> Private->PciIo,
>> EfiPciIoWidthFillUint32,
>> - 0,
>> + Private->FrameBufferVramBarIndex,
>> 0,
>> 0x400000 >> 2,
>> &Color
>> diff --git a/OvmfPkg/QemuVideoDxe/Gop.c b/OvmfPkg/QemuVideoDxe/Gop.c
>> index d490fa7a2e6f..6f542d9eac05 100644
>> --- a/OvmfPkg/QemuVideoDxe/Gop.c
>> +++ b/OvmfPkg/QemuVideoDxe/Gop.c
>> @@ -60,7 +60,7 @@ QemuVideoCompleteModeData (
>>
>> Private->PciIo->GetBarAttributes (
>> Private->PciIo,
>> - 0,
>> + Private->FrameBufferVramBarIndex,
>> NULL,
>> (VOID**) &FrameBufDesc
>> );
>
> For me this is much easier to understand.
I agree. Will do
Thanks,
yuchenlin
>
> ... While we discuss this, I'll go ahead and push the first four
> patches. The code being reverted is dead anyway. I'll report back about
> the commit hashes.
>
> Thanks,
> Laszlo
^ permalink raw reply [flat|nested] 23+ messages in thread
* Re: [PATCH v2 0/5] OvmfPkg: simply use the Bochs interface for vmsvga
2018-11-02 3:23 [PATCH v2 0/5] OvmfPkg: simply use the Bochs interface for vmsvga yuchenlin
` (4 preceding siblings ...)
2018-11-02 3:24 ` [PATCH v2 5/5] OvmfPkg: simply use the Bochs interface for vmsvga yuchenlin
@ 2018-11-05 22:52 ` Laszlo Ersek
2018-11-12 14:16 ` Philippe Mathieu-Daudé
6 siblings, 0 replies; 23+ messages in thread
From: Laszlo Ersek @ 2018-11-05 22:52 UTC (permalink / raw)
To: yuchenlin, edk2-devel
Cc: phil, jordan.l.justen, anthony.perard,
Philippe Mathieu-Daudé
(+PhilMD)
On 11/02/18 04:23, yuchenlin via edk2-devel wrote:
> From: yuchenlin <yuchenlin@synology.com>
>
> In this series, replace the original vmsvga driver to Bochs
> interface.
>
> Simply revert vmsvga driver implementation. After it, use Bochs
> interface for initializing vmsvga.
>
> Because of the PCI BARs difference between std vga and vmsvga.
> We can not simply recognize the "QEMU VMWare SVGA" as the
> QEMU_VIDEO_BOCHS_MMIO variant.
>
> BAR | std vga | vmsvga
> ---------------------------------
> 0 | Framebuffer | I/O space
> 1 | Reserved | Framebuffer
> 2 | MMIO | FIFO
>
> To overcome this problem, we remain variant QEMU_VIDEO_VMWARE_SVGA,
> and use it for:
>
> (1) Get framebuffer from correct PCI BAR
> (2) Prevent using BAR2 for MMIO
>
> We have tested on qemu before and after commit 104bd1dc70 and all
> worked.
>
> Contributed-under: TianoCore Contribution Agreement 1.1
> Signed-off-by: yuchenlin <yuchenlin@synology.com>
>
> Changelog:
> v1 -> v2
> * use 'else' clause (Thanks Philippe).
> * add more comment in revert patches (Thanks Philippe).
> * reorder the revert patches, we should revert the last commit first.
> * use correct framebuffer to ClearScreen.
> * revert VMWare svga definitions.
I'll have to ask for a bit more patience until I come to this series. I
can process only so much email backlog in one day. :) And I don't want
to butcher this review due to fatigue. Meanwhile, thank you both Phils
for the v1 comments! Please comment on v2 as well, if you can!
Laszlo
^ permalink raw reply [flat|nested] 23+ messages in thread* Re: [PATCH v2 0/5] OvmfPkg: simply use the Bochs interface for vmsvga
2018-11-02 3:23 [PATCH v2 0/5] OvmfPkg: simply use the Bochs interface for vmsvga yuchenlin
` (5 preceding siblings ...)
2018-11-05 22:52 ` [PATCH v2 0/5] " Laszlo Ersek
@ 2018-11-12 14:16 ` Philippe Mathieu-Daudé
2018-11-12 15:40 ` Laszlo Ersek
6 siblings, 1 reply; 23+ messages in thread
From: Philippe Mathieu-Daudé @ 2018-11-12 14:16 UTC (permalink / raw)
To: yuchenlin, edk2-devel
Cc: jordan.l.justen, lersek, ard.biesheuvel, anthony.perard,
julien.grall, phil, kraxel
On 2/11/18 4:23, yuchenlin@synology.com wrote:
> From: yuchenlin <yuchenlin@synology.com>
>
> In this series, replace the original vmsvga driver to Bochs
> interface.
>
> Simply revert vmsvga driver implementation. After it, use Bochs
> interface for initializing vmsvga.
>
> Because of the PCI BARs difference between std vga and vmsvga.
> We can not simply recognize the "QEMU VMWare SVGA" as the
> QEMU_VIDEO_BOCHS_MMIO variant.
>
> BAR | std vga | vmsvga
> ---------------------------------
> 0 | Framebuffer | I/O space
> 1 | Reserved | Framebuffer
> 2 | MMIO | FIFO
>
> To overcome this problem, we remain variant QEMU_VIDEO_VMWARE_SVGA,
> and use it for:
>
> (1) Get framebuffer from correct PCI BAR
> (2) Prevent using BAR2 for MMIO
>
> We have tested on qemu before and after commit 104bd1dc70 and all
> worked.
>
> Contributed-under: TianoCore Contribution Agreement 1.1
> Signed-off-by: yuchenlin <yuchenlin@synology.com>
>
> Changelog:
> v1 -> v2
> * use 'else' clause (Thanks Philippe).
> * add more comment in revert patches (Thanks Philippe).
> * reorder the revert patches, we should revert the last commit first.
> * use correct framebuffer to ClearScreen.
> * revert VMWare svga definitions.
>
> yuchenlin (5):
> Revert "OvmfPkg/QemuVideoDxe: list "UnalignedIoInternal.h" in the INF
> file"
> Revert "OvmfPkg/QemuVideoDxe: VMWare SVGA device support"
> Revert "OvmfPkg/QemuVideoDxe: Helper functions for unaligned port
> I/O."
> Revert "OvmfPkg: VMWare SVGA display device register definitions"
> OvmfPkg: simply use the Bochs interface for vmsvga
FWIW Patches 1-4:
Reviewed-by: Philippe Mathieu-Daudé <philmd@redhat.com>
Patch 5 (not yet merged) is still on my TODO because I want to run
detailed testing.
^ permalink raw reply [flat|nested] 23+ messages in thread* Re: [PATCH v2 0/5] OvmfPkg: simply use the Bochs interface for vmsvga
2018-11-12 14:16 ` Philippe Mathieu-Daudé
@ 2018-11-12 15:40 ` Laszlo Ersek
2018-11-15 11:56 ` Laszlo Ersek
0 siblings, 1 reply; 23+ messages in thread
From: Laszlo Ersek @ 2018-11-12 15:40 UTC (permalink / raw)
To: Philippe Mathieu-Daudé, yuchenlin, edk2-devel
Cc: jordan.l.justen, ard.biesheuvel, anthony.perard, julien.grall,
phil, kraxel
On 11/12/18 15:16, Philippe Mathieu-Daudé wrote:
> On 2/11/18 4:23, yuchenlin@synology.com wrote:
>> From: yuchenlin <yuchenlin@synology.com>
>>
>> In this series, replace the original vmsvga driver to Bochs
>> interface.
>>
>> Simply revert vmsvga driver implementation. After it, use Bochs
>> interface for initializing vmsvga.
>>
>> Because of the PCI BARs difference between std vga and vmsvga.
>> We can not simply recognize the "QEMU VMWare SVGA" as the
>> QEMU_VIDEO_BOCHS_MMIO variant.
>>
>> BAR | std vga | vmsvga
>> ---------------------------------
>> 0 | Framebuffer | I/O space
>> 1 | Reserved | Framebuffer
>> 2 | MMIO | FIFO
>>
>> To overcome this problem, we remain variant QEMU_VIDEO_VMWARE_SVGA,
>> and use it for:
>>
>> (1) Get framebuffer from correct PCI BAR
>> (2) Prevent using BAR2 for MMIO
>>
>> We have tested on qemu before and after commit 104bd1dc70 and all
>> worked.
>>
>> Contributed-under: TianoCore Contribution Agreement 1.1
>> Signed-off-by: yuchenlin <yuchenlin@synology.com>
>>
>> Changelog:
>> v1 -> v2
>> * use 'else' clause (Thanks Philippe).
>> * add more comment in revert patches (Thanks Philippe).
>> * reorder the revert patches, we should revert the last commit first.
>> * use correct framebuffer to ClearScreen.
>> * revert VMWare svga definitions.
>>
>> yuchenlin (5):
>> Revert "OvmfPkg/QemuVideoDxe: list "UnalignedIoInternal.h" in the INF
>> file"
>> Revert "OvmfPkg/QemuVideoDxe: VMWare SVGA device support"
>> Revert "OvmfPkg/QemuVideoDxe: Helper functions for unaligned port
>> I/O."
>> Revert "OvmfPkg: VMWare SVGA display device register definitions"
>> OvmfPkg: simply use the Bochs interface for vmsvga
>
> FWIW Patches 1-4:
> Reviewed-by: Philippe Mathieu-Daudé <philmd@redhat.com>
Thanks!
> Patch 5 (not yet merged) is still on my TODO because I want to run
> detailed testing.
Thanks for that too. :) Please remember to use the v3 posting for that:
[edk2] [PATCH v3] OvmfPkg: simply use the Bochs interface for vmsvga
(msgid: <20181107034713.24907-1-yuchenlin@synology.com>)
Cheers!
Laszlo
^ permalink raw reply [flat|nested] 23+ messages in thread
* Re: [PATCH v2 0/5] OvmfPkg: simply use the Bochs interface for vmsvga
2018-11-12 15:40 ` Laszlo Ersek
@ 2018-11-15 11:56 ` Laszlo Ersek
0 siblings, 0 replies; 23+ messages in thread
From: Laszlo Ersek @ 2018-11-15 11:56 UTC (permalink / raw)
To: yuchenlin
Cc: Philippe Mathieu-Daudé, edk2-devel, phil, jordan.l.justen,
anthony.perard
On 11/12/18 16:40, Laszlo Ersek wrote:
> On 11/12/18 15:16, Philippe Mathieu-Daudé wrote:
>> On 2/11/18 4:23, yuchenlin@synology.com wrote:
>>> From: yuchenlin <yuchenlin@synology.com>
>>>
>>> In this series, replace the original vmsvga driver to Bochs
>>> interface.
>>>
>>> Simply revert vmsvga driver implementation. After it, use Bochs
>>> interface for initializing vmsvga.
>>>
>>> Because of the PCI BARs difference between std vga and vmsvga.
>>> We can not simply recognize the "QEMU VMWare SVGA" as the
>>> QEMU_VIDEO_BOCHS_MMIO variant.
>>>
>>> BAR | std vga | vmsvga
>>> ---------------------------------
>>> 0 | Framebuffer | I/O space
>>> 1 | Reserved | Framebuffer
>>> 2 | MMIO | FIFO
>>>
>>> To overcome this problem, we remain variant QEMU_VIDEO_VMWARE_SVGA,
>>> and use it for:
>>>
>>> (1) Get framebuffer from correct PCI BAR
>>> (2) Prevent using BAR2 for MMIO
>>>
>>> We have tested on qemu before and after commit 104bd1dc70 and all
>>> worked.
>>>
>>> Contributed-under: TianoCore Contribution Agreement 1.1
>>> Signed-off-by: yuchenlin <yuchenlin@synology.com>
>>>
>>> Changelog:
>>> v1 -> v2
>>> * use 'else' clause (Thanks Philippe).
>>> * add more comment in revert patches (Thanks Philippe).
>>> * reorder the revert patches, we should revert the last commit first.
>>> * use correct framebuffer to ClearScreen.
>>> * revert VMWare svga definitions.
>>>
>>> yuchenlin (5):
>>> Revert "OvmfPkg/QemuVideoDxe: list "UnalignedIoInternal.h" in the INF
>>> file"
>>> Revert "OvmfPkg/QemuVideoDxe: VMWare SVGA device support"
>>> Revert "OvmfPkg/QemuVideoDxe: Helper functions for unaligned port
>>> I/O."
>>> Revert "OvmfPkg: VMWare SVGA display device register definitions"
>>> OvmfPkg: simply use the Bochs interface for vmsvga
>>
>> FWIW Patches 1-4:
>> Reviewed-by: Philippe Mathieu-Daudé <philmd@redhat.com>
I could apply the first 4 patches now (commit 85588389222a has been
tagged as "edk2-stable201811"); however, I'd prefer to push all 5 in one go:
>> Patch 5 (not yet merged) is still on my TODO because I want to run
>> detailed testing.
>
> Thanks for that too. :) Please remember to use the v3 posting for that:
>
> [edk2] [PATCH v3] OvmfPkg: simply use the Bochs interface for vmsvga
>
> (msgid: <20181107034713.24907-1-yuchenlin@synology.com>)
with Phil's upcoming feedback attached to the last -- 5th -- patch too.
Thanks
Laszlo
^ permalink raw reply [flat|nested] 23+ messages in thread