From f10ae923665fae7dff10a58a70f83e4be3409436 Mon Sep 17 00:00:00 2001 From: Laszlo Ersek Date: Sat, 26 Aug 2017 18:12:15 +0200 Subject: [PATCH] OvmfPkg/VirtioGpuDxe: map backing store to bus master device address VirtioGpuDxe is a UEFI Bus driver (not a Device driver). This is because a UEFI graphics driver is expected to produce its GraphicsOutput protocol instance(s) on new child handle(s) of the video controller handle, one child handle (plus GOP) per video output (or, one child handle plus GOP per combination of multiple video outputs). In VirtioGpuDxe, we support a single VirtIo GPU head (scanout), namely head#0. This means that, with regard to a specific VirtIo GPU device, the driver may be in one of three states, at any time: [1] VirtioGpuDxe has not bound the device at all, [2] VirtioGpuDxe has bound the device, but not produced the sole child handle for head#0, [3] VirtioGpuDxe has bound the device, and produced the sole child handle for head#0, with a GOP instance on the child handle. (Which state the driver is in wrt. a given VirtIo GPU device depends on the VirtioGpuDriverBindingStart() / VirtioGpuDriverBindingStop() invocations issued by the ConnectController() / DisconnectController() boot services. In turn those come from BDS or e.g. the UEFI shell.) The concept of "current video mode" is technically tied to the GOP (i.e., the child handle, state [3] only), not the VirtIo GPU controller handle. This is why we manage the storage that backs the current video mode in our EFI_GRAPHICS_OUTPUT_PROTOCOL.SetMode() member implementation. GopSetMode() is first called *internally*, when we enter state [3] (that is, when we produce the child handle + GOP on it): VirtioGpuDriverBindingStart() [DriverBinding.c] InitVgpuGop() [DriverBinding.c] VgpuGop->Gop.SetMode() [Gop.c] When this happens, we allocate the backing store *without* having a preexistent backing store (due to no preexistent video mode and GOP). Skipping VirtIo GPU details not relevant for this patch, we just note that the backing store is exposed *permanently* to the VirtIo GPU device, with the RESOURCE_ATTACH_BACKING command. When external clients call the EFI_GRAPHICS_OUTPUT_PROTOCOL.Blt() member function -- called GopBlt() in this driver --, in state [3], the function operates on the backing store, and sends only small messages to the VirtIo GPU device. When external clients call GopSetMode() for switching between video modes -- in state [3] --, then - a new backing store is allocated and exposed to the device (attached to a new host-side VirtIo GPU resource), - head#0 is flipped to the new backing store, - on success, the ReleaseGopResources() function both detaches the previous backing store from the VirtIo GPU device, an releases it. The new backing store address and size are saved in our GOP object. (In other words, we "commit" to the new video mode.) When the DisconnectController() boot service asks us to leave state [3] -- we can leave it directly only for state [2] --, then the ReleaseGopResources() function is called on a different path: VirtioGpuDriverBindingStop() [DriverBinding.c] UninitVgpuGop() [DriverBinding.c] ReleaseGopResources() [Gop.c] In this case, the backing store being released is still in use (we're not leaving it for a new mode -- head#0 has not been flipped "away" from it), so in ReleaseGopResources() we disable head#0 first. (The ReleaseGopResources() function is called the same way on the error path in InitVgpuGop(), if the first -- internal -- VgpuGop->Gop.SetMode() call succeeds, but the rest of InitVgpuGop() fails.) Based on the above, for IOMMU-compatibility, - in GopSetMode(), don't just allocate, but also map the backing store of the nascent video mode to a device address, for bus master common buffer operation, - (the VirtioGpuAllocateZeroAndMapBackingStore() helper function introduced in the last patch takes care of zeroing internally,) - pass the device address to the VirtIo GPU device in the RESOURCE_ATTACH_BACKING command, - if GopSetMode() succeeds, save the mapping token, - if GopSetMode() fails, don't just free but also unmap the still-born backing store, - in ReleaseGopResources(), don't just free but also unmap the backing store -- which is the previous backing store if we're mode-switching, and the current backing store if we're leaving state [3]. Finally, ExitBootServices() may be called when the driver is in either state [1], [2] or [3], wrt. a given VirtIo GPU device. (Of course we are only notified in states [2] and [3].) If we get the notification in state [3], then the current video mode's backing store has to be unmapped, but not released. (We must not change the UEFI memory map.) Cc: Ard Biesheuvel Cc: Brijesh Singh Cc: Jordan Justen Cc: Tom Lendacky Contributed-under: TianoCore Contribution Agreement 1.1 Signed-off-by: Laszlo Ersek Tested-by: Brijesh Singh --- OvmfPkg/VirtioGpuDxe/Commands.c | 21 ++++++++++++ OvmfPkg/VirtioGpuDxe/Gop.c | 57 +++++++++++++++++++++----------- OvmfPkg/VirtioGpuDxe/VirtioGpu.h | 7 ++++ 3 files changed, 65 insertions(+), 20 deletions(-) diff --git a/OvmfPkg/VirtioGpuDxe/Commands.c b/OvmfPkg/VirtioGpuDxe/Commands.c index 595a3717d9..db5bdbca4b 100644 --- a/OvmfPkg/VirtioGpuDxe/Commands.c +++ b/OvmfPkg/VirtioGpuDxe/Commands.c @@ -355,6 +355,27 @@ VirtioGpuExitBoot ( VgpuDev = Context; VgpuDev->VirtIo->SetDeviceStatus (VgpuDev->VirtIo, 0); + + // + // If VirtioGpuDriverBindingStart() and VirtioGpuDriverBindingStop() have + // been called thus far in such a sequence that right now our (sole) child + // handle exists -- with the GOP on it standing for head (scanout) #0 --, + // then we have to unmap the current video mode's backing store. + // + if (VgpuDev->Child != NULL) { + // + // The current video mode is guaranteed to have a valid and mapped backing + // store, due to the first Gop.SetMode() call, made internally in + // InitVgpuGop(). + // + ASSERT (VgpuDev->Child->BackingStore != NULL); + + VgpuDev->VirtIo->UnmapSharedBuffer ( + VgpuDev->VirtIo, + VgpuDev->Child->BackingStoreMap + ); + } + VgpuDev->VirtIo->UnmapSharedBuffer (VgpuDev->VirtIo, VgpuDev->RingMap); } diff --git a/OvmfPkg/VirtioGpuDxe/Gop.c b/OvmfPkg/VirtioGpuDxe/Gop.c index 507e1a770d..936e181e8a 100644 --- a/OvmfPkg/VirtioGpuDxe/Gop.c +++ b/OvmfPkg/VirtioGpuDxe/Gop.c @@ -114,11 +114,17 @@ ReleaseGopResources ( } // - // Release backing pages. - // - FreePages (VgpuGop->BackingStore, VgpuGop->NumberOfPages); + // Unmap and release backing pages. + // + VirtioGpuUnmapAndFreeBackingStore ( + VgpuGop->ParentBus, // VgpuDev + VgpuGop->NumberOfPages, // NumberOfPages + VgpuGop->BackingStore, // HostAddress + VgpuGop->BackingStoreMap // Mapping + ); VgpuGop->BackingStore = NULL; VgpuGop->NumberOfPages = 0; + VgpuGop->BackingStoreMap = NULL; // // Destroy the currently used 2D host resource. @@ -230,11 +236,14 @@ GopSetMode ( IN UINT32 ModeNumber ) { - VGPU_GOP *VgpuGop; - UINT32 NewResourceId; - UINTN NewNumberOfBytes; - UINTN NewNumberOfPages; - VOID *NewBackingStore; + VGPU_GOP *VgpuGop; + UINT32 NewResourceId; + UINTN NewNumberOfBytes; + UINTN NewNumberOfPages; + VOID *NewBackingStore; + EFI_PHYSICAL_ADDRESS NewBackingStoreDeviceAddress; + VOID *NewBackingStoreMap; + EFI_STATUS Status; EFI_STATUS Status2; @@ -293,20 +302,22 @@ GopSetMode ( } // - // Allocate guest backing store. + // Allocate, zero and map guest backing store, for bus master common buffer + // operation. // NewNumberOfBytes = mGopResolutions[ModeNumber].Width * mGopResolutions[ModeNumber].Height * sizeof (UINT32); NewNumberOfPages = EFI_SIZE_TO_PAGES (NewNumberOfBytes); - NewBackingStore = AllocatePages (NewNumberOfPages); - if (NewBackingStore == NULL) { - Status = EFI_OUT_OF_RESOURCES; + Status = VirtioGpuAllocateZeroAndMapBackingStore ( + VgpuGop->ParentBus, // VgpuDev + NewNumberOfPages, // NumberOfPages + &NewBackingStore, // HostAddress + &NewBackingStoreDeviceAddress, // DeviceAddress + &NewBackingStoreMap // Mapping + ); + if (EFI_ERROR (Status)) { goto DestroyHostResource; } - // - // Fill visible part of backing store with black. - // - ZeroMem (NewBackingStore, NewNumberOfBytes); // // Attach backing store to the host resource. @@ -314,11 +325,11 @@ GopSetMode ( Status = VirtioGpuResourceAttachBacking ( VgpuGop->ParentBus, // VgpuDev NewResourceId, // ResourceId - (UINTN)NewBackingStore, // BackingStoreDeviceAddress + NewBackingStoreDeviceAddress, // BackingStoreDeviceAddress NewNumberOfPages // NumberOfPages ); if (EFI_ERROR (Status)) { - goto FreeBackingStore; + goto UnmapAndFreeBackingStore; } // @@ -391,6 +402,7 @@ GopSetMode ( VgpuGop->ResourceId = NewResourceId; VgpuGop->BackingStore = NewBackingStore; VgpuGop->NumberOfPages = NewNumberOfPages; + VgpuGop->BackingStoreMap = NewBackingStoreMap; // // Populate Mode and ModeInfo (mutable fields only). @@ -409,8 +421,13 @@ DetachBackingStore: CpuDeadLoop (); } -FreeBackingStore: - FreePages (NewBackingStore, NewNumberOfPages); +UnmapAndFreeBackingStore: + VirtioGpuUnmapAndFreeBackingStore ( + VgpuGop->ParentBus, // VgpuDev + NewNumberOfPages, // NumberOfPages + NewBackingStore, // HostAddress + NewBackingStoreMap // Mapping + ); DestroyHostResource: Status2 = VirtioGpuResourceUnref (VgpuGop->ParentBus, NewResourceId); diff --git a/OvmfPkg/VirtioGpuDxe/VirtioGpu.h b/OvmfPkg/VirtioGpuDxe/VirtioGpu.h index 65b1bd6088..73893ccb56 100644 --- a/OvmfPkg/VirtioGpuDxe/VirtioGpu.h +++ b/OvmfPkg/VirtioGpuDxe/VirtioGpu.h @@ -150,6 +150,13 @@ struct VGPU_GOP_STRUCT { // UINT32 *BackingStore; UINTN NumberOfPages; + + // + // Token associated with BackingStore's mapping for bus master common + // buffer operation. BackingStoreMap is valid if, and only if, + // BackingStore is non-NULL. + // + VOID *BackingStoreMap; }; // -- 2.39.2