* [PATCH 0/6] OvmfPkg/VirtioGpuDxe: map system memory addresses to device addresses
@ 2017-08-28 13:24 Laszlo Ersek
2017-08-28 13:24 ` [PATCH 1/6] OvmfPkg/VirtioGpuDxe: map VRING for bus master common buffer operation Laszlo Ersek
` (6 more replies)
0 siblings, 7 replies; 11+ messages in thread
From: Laszlo Ersek @ 2017-08-28 13:24 UTC (permalink / raw)
To: edk2-devel-01; +Cc: Ard Biesheuvel, Brijesh Singh, Jordan Justen, Tom Lendacky
Repo: https://github.com/lersek/edk2.git
Branch: virtio_gpu_map
This series brings IOMMU support to VirtioGpuDxe. The interesting part
is patch#5.
I formatted the patches with function context, for easier review.
I regression-tested this series in various environments; IA32, X64,
AARCH64. My access to a SEV machine is still in the making, so I
couldn't test the patches on SEV.
Brijesh, if you could test the series for me on SEV, that would be
great. I also plan to test it on SEV, once my access is established.
Cc: Ard Biesheuvel <ard.biesheuvel@linaro.org>
Cc: Brijesh Singh <brijesh.singh@amd.com>
Cc: Jordan Justen <jordan.l.justen@intel.com>
Cc: Tom Lendacky <thomas.lendacky@amd.com>
Thanks,
Laszlo
Laszlo Ersek (6):
OvmfPkg/VirtioGpuDxe: map VRING for bus master common buffer operation
OvmfPkg/VirtioGpuDxe: map virtio GPU command objects to device
addresses
OvmfPkg/VirtioGpuDxe: take EFI_PHYSICAL_ADDRESS in
ResourceAttachBacking()
OvmfPkg/VirtioGpuDxe: helpers for backing store
(de)allocation+(un)mapping
OvmfPkg/VirtioGpuDxe: map backing store to bus master device address
OvmfPkg/VirtioGpuDxe: negotiate VIRTIO_F_IOMMU_PLATFORM
OvmfPkg/VirtioGpuDxe/VirtioGpu.h | 89 ++++++-
OvmfPkg/VirtioGpuDxe/Commands.c | 263 ++++++++++++++++++--
OvmfPkg/VirtioGpuDxe/DriverBinding.c | 1 -
OvmfPkg/VirtioGpuDxe/Gop.c | 62 +++--
4 files changed, 370 insertions(+), 45 deletions(-)
--
2.14.1.3.gb7cf6e02401b
^ permalink raw reply [flat|nested] 11+ messages in thread
* [PATCH 1/6] OvmfPkg/VirtioGpuDxe: map VRING for bus master common buffer operation
2017-08-28 13:24 [PATCH 0/6] OvmfPkg/VirtioGpuDxe: map system memory addresses to device addresses Laszlo Ersek
@ 2017-08-28 13:24 ` Laszlo Ersek
2017-08-28 13:24 ` [PATCH 2/6] OvmfPkg/VirtioGpuDxe: map virtio GPU command objects to device addresses Laszlo Ersek
` (5 subsequent siblings)
6 siblings, 0 replies; 11+ messages in thread
From: Laszlo Ersek @ 2017-08-28 13:24 UTC (permalink / raw)
To: edk2-devel-01; +Cc: Ard Biesheuvel, Brijesh Singh, Jordan Justen, Tom Lendacky
VirtioGpuDxe uses one virtio ring, for VIRTIO_GPU_CONTROL_QUEUE.
Map it for bus master common buffer operation with VirtioRingMap(), so
that it can be accessed equally by both guest and hypervisor even if an
IOMMU is used. (VirtioRingInit() already allocates the ring suitably for
this, see commit b0338c53297c, "OvmfPkg/VirtioLib: alloc VRING buffer with
AllocateSharedPages()", 2017-08-23).
Pass the resultant translation offset ("RingBaseShift"), from system
memory address to bus master device address, to VIRTIO_SET_QUEUE_ADDRESS.
Unmap the ring in all contexts where the ring becomes unused (these
contexts are mutually exclusive):
- in VirtioGpuInit(): the ring has been mapped, but we cannot complete the
virtio initialization for another reason,
- in VirtioGpuUninit(): the virtio initialization has succeeded, but
VirtioGpuDriverBindingStart() fails for another reason, or
VirtioGpuDriverBindingStop() unbinds the device after use,
- in VirtioGpuExitBoot(): ExitBootServices() is called after
VirtioGpuDriverBindingStart() has successfully bound the device.
(Unmapping the ring does not change the UEFI memory map.)
Cc: Ard Biesheuvel <ard.biesheuvel@linaro.org>
Cc: Brijesh Singh <brijesh.singh@amd.com>
Cc: Jordan Justen <jordan.l.justen@intel.com>
Cc: Tom Lendacky <thomas.lendacky@amd.com>
Contributed-under: TianoCore Contribution Agreement 1.1
Signed-off-by: Laszlo Ersek <lersek@redhat.com>
---
OvmfPkg/VirtioGpuDxe/VirtioGpu.h | 6 +++++
OvmfPkg/VirtioGpuDxe/Commands.c | 27 +++++++++++++++++---
2 files changed, 30 insertions(+), 3 deletions(-)
diff --git a/OvmfPkg/VirtioGpuDxe/VirtioGpu.h b/OvmfPkg/VirtioGpuDxe/VirtioGpu.h
index 078b7d44d83e..193e932e1430 100644
--- a/OvmfPkg/VirtioGpuDxe/VirtioGpu.h
+++ b/OvmfPkg/VirtioGpuDxe/VirtioGpu.h
@@ -40,46 +40,52 @@ typedef struct VGPU_GOP_STRUCT VGPU_GOP;
typedef struct {
//
// VirtIo represents access to the Virtio GPU device. Never NULL.
//
VIRTIO_DEVICE_PROTOCOL *VirtIo;
//
// BusName carries a customized name for
// EFI_COMPONENT_NAME2_PROTOCOL.GetControllerName(). It is expressed in table
// form because it can theoretically support several languages. Never NULL.
//
EFI_UNICODE_STRING_TABLE *BusName;
//
// VirtIo ring used for VirtIo communication.
//
VRING Ring;
+ //
+ // Token associated with Ring's mapping for bus master common buffer
+ // operation, from VirtioRingMap().
+ //
+ VOID *RingMap;
+
//
// Event to be signaled at ExitBootServices().
//
EFI_EVENT ExitBoot;
//
// Common running counter for all VirtIo GPU requests that ask for fencing.
//
UINT64 FenceId;
//
// The Child field references the GOP wrapper structure. If this pointer is
// NULL, then the hybrid driver has bound (i.e., started) the
// VIRTIO_DEVICE_PROTOCOL controller without producing the child GOP
// controller (that is, after Start() was called with RemainingDevicePath
// pointing to and End of Device Path node). Child can be created and
// destroyed, even repeatedly, independently of VGPU_DEV.
//
// In practice, this field represents the single head (scanout) that we
// support.
//
VGPU_GOP *Child;
} VGPU_DEV;
//
// The Graphics Output Protocol wrapper structure.
//
#define VGPU_GOP_SIG SIGNATURE_64 ('V', 'G', 'P', 'U', '_', 'G', 'O', 'P')
diff --git a/OvmfPkg/VirtioGpuDxe/Commands.c b/OvmfPkg/VirtioGpuDxe/Commands.c
index 5cb003161207..4e19bac606ee 100644
--- a/OvmfPkg/VirtioGpuDxe/Commands.c
+++ b/OvmfPkg/VirtioGpuDxe/Commands.c
@@ -39,119 +39,138 @@ EFI_STATUS
VirtioGpuInit (
IN OUT VGPU_DEV *VgpuDev
)
{
UINT8 NextDevStat;
EFI_STATUS Status;
UINT64 Features;
UINT16 QueueSize;
+ UINT64 RingBaseShift;
//
// Execute virtio-v1.0-cs04, 3.1.1 Driver Requirements: Device
// Initialization.
//
// 1. Reset the device.
//
NextDevStat = 0;
Status = VgpuDev->VirtIo->SetDeviceStatus (VgpuDev->VirtIo, NextDevStat);
if (EFI_ERROR (Status)) {
goto Failed;
}
//
// 2. Set the ACKNOWLEDGE status bit [...]
//
NextDevStat |= VSTAT_ACK;
Status = VgpuDev->VirtIo->SetDeviceStatus (VgpuDev->VirtIo, NextDevStat);
if (EFI_ERROR (Status)) {
goto Failed;
}
//
// 3. Set the DRIVER status bit [...]
//
NextDevStat |= VSTAT_DRIVER;
Status = VgpuDev->VirtIo->SetDeviceStatus (VgpuDev->VirtIo, NextDevStat);
if (EFI_ERROR (Status)) {
goto Failed;
}
//
// 4. Read device feature bits...
//
Status = VgpuDev->VirtIo->GetDeviceFeatures (VgpuDev->VirtIo, &Features);
if (EFI_ERROR (Status)) {
goto Failed;
}
if ((Features & VIRTIO_F_VERSION_1) == 0) {
Status = EFI_UNSUPPORTED;
goto Failed;
}
//
// We only want the most basic 2D features.
//
Features &= VIRTIO_F_VERSION_1;
//
// ... and write the subset of feature bits understood by the [...] driver to
// the device. [...]
// 5. Set the FEATURES_OK status bit.
// 6. Re-read device status to ensure the FEATURES_OK bit is still set [...]
//
Status = Virtio10WriteFeatures (VgpuDev->VirtIo, Features, &NextDevStat);
if (EFI_ERROR (Status)) {
goto Failed;
}
//
// 7. Perform device-specific setup, including discovery of virtqueues for
// the device [...]
//
Status = VgpuDev->VirtIo->SetQueueSel (VgpuDev->VirtIo,
VIRTIO_GPU_CONTROL_QUEUE);
if (EFI_ERROR (Status)) {
goto Failed;
}
Status = VgpuDev->VirtIo->GetQueueNumMax (VgpuDev->VirtIo, &QueueSize);
if (EFI_ERROR (Status)) {
goto Failed;
}
//
// We implement each VirtIo GPU command that we use with two descriptors:
// request, response.
//
if (QueueSize < 2) {
Status = EFI_UNSUPPORTED;
goto Failed;
}
//
// [...] population of virtqueues [...]
//
Status = VirtioRingInit (VgpuDev->VirtIo, QueueSize, &VgpuDev->Ring);
if (EFI_ERROR (Status)) {
goto Failed;
}
+ //
+ // If anything fails from here on, we have to release the ring.
+ //
+ Status = VirtioRingMap (
+ VgpuDev->VirtIo,
+ &VgpuDev->Ring,
+ &RingBaseShift,
+ &VgpuDev->RingMap
+ );
+ if (EFI_ERROR (Status)) {
+ goto ReleaseQueue;
+ }
+ //
+ // If anything fails from here on, we have to unmap the ring.
+ //
Status = VgpuDev->VirtIo->SetQueueAddress (
VgpuDev->VirtIo,
&VgpuDev->Ring,
- 0
+ RingBaseShift
);
if (EFI_ERROR (Status)) {
- goto ReleaseQueue;
+ goto UnmapQueue;
}
//
// 8. Set the DRIVER_OK status bit.
//
NextDevStat |= VSTAT_DRIVER_OK;
Status = VgpuDev->VirtIo->SetDeviceStatus (VgpuDev->VirtIo, NextDevStat);
if (EFI_ERROR (Status)) {
- goto ReleaseQueue;
+ goto UnmapQueue;
}
return EFI_SUCCESS;
+UnmapQueue:
+ VgpuDev->VirtIo->UnmapSharedBuffer (VgpuDev->VirtIo, VgpuDev->RingMap);
+
ReleaseQueue:
VirtioRingUninit (VgpuDev->VirtIo, &VgpuDev->Ring);
@@ -182,25 +201,26 @@ VOID
VirtioGpuUninit (
IN OUT VGPU_DEV *VgpuDev
)
{
//
// Resetting the VirtIo device makes it release its resources and forget its
// configuration.
//
VgpuDev->VirtIo->SetDeviceStatus (VgpuDev->VirtIo, 0);
+ VgpuDev->VirtIo->UnmapSharedBuffer (VgpuDev->VirtIo, VgpuDev->RingMap);
VirtioRingUninit (VgpuDev->VirtIo, &VgpuDev->Ring);
}
/**
EFI_EVENT_NOTIFY function for the VGPU_DEV.ExitBoot event. It resets the
VirtIo device, causing it to release its resources and to forget its
configuration.
This function may only be called (that is, VGPU_DEV.ExitBoot may only be
signaled) after VirtioGpuInit() returns and before VirtioGpuUninit() is
called.
@param[in] Event Event whose notification function is being invoked.
@param[in] Context Pointer to the associated VGPU_DEV object.
**/
@@ -209,53 +229,54 @@ EFIAPI
VirtioGpuExitBoot (
IN EFI_EVENT Event,
IN VOID *Context
)
{
VGPU_DEV *VgpuDev;
VgpuDev = Context;
VgpuDev->VirtIo->SetDeviceStatus (VgpuDev->VirtIo, 0);
+ VgpuDev->VirtIo->UnmapSharedBuffer (VgpuDev->VirtIo, VgpuDev->RingMap);
}
/**
Internal utility function that sends a request to the VirtIo GPU device
model, awaits the answer from the host, and returns a status.
@param[in,out] VgpuDev The VGPU_DEV object that represents the VirtIo GPU
device. The caller is responsible to have
successfully invoked VirtioGpuInit() on VgpuDev
previously, while VirtioGpuUninit() must not have
been called on VgpuDev.
@param[in] RequestType The type of the request. The caller is responsible
for providing a VirtioGpuCmd* RequestType which, on
success, elicits a VirtioGpuRespOkNodata response
from the host.
@param[in] Fence Whether to enable fencing for this request. Fencing
forces the host to complete the command before
producing a response. If Fence is TRUE, then
VgpuDev->FenceId is consumed, and incremented.
@param[in,out] Header Pointer to the caller-allocated request object. The
request must start with VIRTIO_GPU_CONTROL_HEADER.
This function overwrites all fields of Header before
submitting the request to the host:
- it sets Type from RequestType,
- it sets Flags and FenceId based on Fence,
- it zeroes CtxId and Padding.
@param[in] RequestSize Size of the entire caller-allocated request object,
including the leading VIRTIO_GPU_CONTROL_HEADER.
@retval EFI_SUCCESS Operation successful.
@retval EFI_DEVICE_ERROR The host rejected the request. The host error
code has been logged on the EFI_D_ERROR level.
@return Codes for unexpected errors in VirtIo
messaging.
**/
--
2.14.1.3.gb7cf6e02401b
^ permalink raw reply related [flat|nested] 11+ messages in thread
* [PATCH 2/6] OvmfPkg/VirtioGpuDxe: map virtio GPU command objects to device addresses
2017-08-28 13:24 [PATCH 0/6] OvmfPkg/VirtioGpuDxe: map system memory addresses to device addresses Laszlo Ersek
2017-08-28 13:24 ` [PATCH 1/6] OvmfPkg/VirtioGpuDxe: map VRING for bus master common buffer operation Laszlo Ersek
@ 2017-08-28 13:24 ` Laszlo Ersek
2017-08-28 13:24 ` [PATCH 3/6] OvmfPkg/VirtioGpuDxe: take EFI_PHYSICAL_ADDRESS in ResourceAttachBacking() Laszlo Ersek
` (4 subsequent siblings)
6 siblings, 0 replies; 11+ messages in thread
From: Laszlo Ersek @ 2017-08-28 13:24 UTC (permalink / raw)
To: edk2-devel-01; +Cc: Ard Biesheuvel, Brijesh Singh, Jordan Justen, Tom Lendacky
Every virtio GPU command used by VirtioGpuDxe is synchronous and formatted
as a two-descriptor chain: request, response. The internal workhorse
function that all the command-specific functions call for such messaging
is VirtioGpuSendCommand().
In VirtioGpuSendCommand(), map the request from system memory to bus
master device address for BusMasterRead operation, and map the response
from system memory to bus master device address for BusMasterWrite
operation.
Pass the bus master device addresses to VirtioAppendDesc(). (See also
commit 4b725858de68, "OvmfPkg/VirtioLib: change the parameter of
VirtioAppendDesc() to UINT64", 2017-08-23.)
Cc: Ard Biesheuvel <ard.biesheuvel@linaro.org>
Cc: Brijesh Singh <brijesh.singh@amd.com>
Cc: Jordan Justen <jordan.l.justen@intel.com>
Cc: Tom Lendacky <thomas.lendacky@amd.com>
Contributed-under: TianoCore Contribution Agreement 1.1
Signed-off-by: Laszlo Ersek <lersek@redhat.com>
---
OvmfPkg/VirtioGpuDxe/Commands.c | 83 ++++++++++++++++++--
1 file changed, 75 insertions(+), 8 deletions(-)
diff --git a/OvmfPkg/VirtioGpuDxe/Commands.c b/OvmfPkg/VirtioGpuDxe/Commands.c
index 4e19bac606ee..bdedea1df6a7 100644
--- a/OvmfPkg/VirtioGpuDxe/Commands.c
+++ b/OvmfPkg/VirtioGpuDxe/Commands.c
@@ -229,148 +229,215 @@ EFIAPI
VirtioGpuExitBoot (
IN EFI_EVENT Event,
IN VOID *Context
)
{
VGPU_DEV *VgpuDev;
VgpuDev = Context;
VgpuDev->VirtIo->SetDeviceStatus (VgpuDev->VirtIo, 0);
VgpuDev->VirtIo->UnmapSharedBuffer (VgpuDev->VirtIo, VgpuDev->RingMap);
}
/**
Internal utility function that sends a request to the VirtIo GPU device
model, awaits the answer from the host, and returns a status.
@param[in,out] VgpuDev The VGPU_DEV object that represents the VirtIo GPU
device. The caller is responsible to have
successfully invoked VirtioGpuInit() on VgpuDev
previously, while VirtioGpuUninit() must not have
been called on VgpuDev.
@param[in] RequestType The type of the request. The caller is responsible
for providing a VirtioGpuCmd* RequestType which, on
success, elicits a VirtioGpuRespOkNodata response
from the host.
@param[in] Fence Whether to enable fencing for this request. Fencing
forces the host to complete the command before
producing a response. If Fence is TRUE, then
VgpuDev->FenceId is consumed, and incremented.
@param[in,out] Header Pointer to the caller-allocated request object. The
request must start with VIRTIO_GPU_CONTROL_HEADER.
This function overwrites all fields of Header before
submitting the request to the host:
- it sets Type from RequestType,
- it sets Flags and FenceId based on Fence,
- it zeroes CtxId and Padding.
@param[in] RequestSize Size of the entire caller-allocated request object,
including the leading VIRTIO_GPU_CONTROL_HEADER.
@retval EFI_SUCCESS Operation successful.
@retval EFI_DEVICE_ERROR The host rejected the request. The host error
code has been logged on the EFI_D_ERROR level.
@return Codes for unexpected errors in VirtIo
- messaging.
+ messaging, or request/response
+ mapping/unmapping.
**/
STATIC
EFI_STATUS
VirtioGpuSendCommand (
IN OUT VGPU_DEV *VgpuDev,
IN VIRTIO_GPU_CONTROL_TYPE RequestType,
IN BOOLEAN Fence,
IN OUT volatile VIRTIO_GPU_CONTROL_HEADER *Header,
IN UINTN RequestSize
)
{
DESC_INDICES Indices;
volatile VIRTIO_GPU_CONTROL_HEADER Response;
EFI_STATUS Status;
UINT32 ResponseSize;
+ EFI_PHYSICAL_ADDRESS RequestDeviceAddress;
+ VOID *RequestMap;
+ EFI_PHYSICAL_ADDRESS ResponseDeviceAddress;
+ VOID *ResponseMap;
//
// Initialize Header.
//
Header->Type = RequestType;
if (Fence) {
Header->Flags = VIRTIO_GPU_FLAG_FENCE;
Header->FenceId = VgpuDev->FenceId++;
} else {
Header->Flags = 0;
Header->FenceId = 0;
}
Header->CtxId = 0;
Header->Padding = 0;
ASSERT (RequestSize >= sizeof *Header);
ASSERT (RequestSize <= MAX_UINT32);
+ //
+ // Map request and response to bus master device addresses.
+ //
+ Status = VirtioMapAllBytesInSharedBuffer (
+ VgpuDev->VirtIo,
+ VirtioOperationBusMasterRead,
+ (VOID *)Header,
+ RequestSize,
+ &RequestDeviceAddress,
+ &RequestMap
+ );
+ if (EFI_ERROR (Status)) {
+ return Status;
+ }
+ Status = VirtioMapAllBytesInSharedBuffer (
+ VgpuDev->VirtIo,
+ VirtioOperationBusMasterWrite,
+ (VOID *)&Response,
+ sizeof Response,
+ &ResponseDeviceAddress,
+ &ResponseMap
+ );
+ if (EFI_ERROR (Status)) {
+ goto UnmapRequest;
+ }
+
//
// Compose the descriptor chain.
//
VirtioPrepare (&VgpuDev->Ring, &Indices);
- VirtioAppendDesc (&VgpuDev->Ring, (UINTN)Header, (UINT32)RequestSize,
- VRING_DESC_F_NEXT, &Indices);
- VirtioAppendDesc (&VgpuDev->Ring, (UINTN)&Response, sizeof Response,
- VRING_DESC_F_WRITE, &Indices);
+ VirtioAppendDesc (
+ &VgpuDev->Ring,
+ RequestDeviceAddress,
+ (UINT32)RequestSize,
+ VRING_DESC_F_NEXT,
+ &Indices
+ );
+ VirtioAppendDesc (
+ &VgpuDev->Ring,
+ ResponseDeviceAddress,
+ (UINT32)sizeof Response,
+ VRING_DESC_F_WRITE,
+ &Indices
+ );
//
// Send the command.
//
Status = VirtioFlush (VgpuDev->VirtIo, VIRTIO_GPU_CONTROL_QUEUE,
&VgpuDev->Ring, &Indices, &ResponseSize);
if (EFI_ERROR (Status)) {
- return Status;
+ goto UnmapResponse;
}
//
- // Parse the response.
+ // Verify response size.
//
if (ResponseSize != sizeof Response) {
DEBUG ((EFI_D_ERROR, "%a: malformed response to Request=0x%x\n",
__FUNCTION__, (UINT32)RequestType));
- return EFI_PROTOCOL_ERROR;
+ Status = EFI_PROTOCOL_ERROR;
+ goto UnmapResponse;
}
+ //
+ // Unmap response and request, in reverse order of mapping. On error, the
+ // respective mapping is invalidated anyway, only the data may not have been
+ // committed to system memory (in case of VirtioOperationBusMasterWrite).
+ //
+ Status = VgpuDev->VirtIo->UnmapSharedBuffer (VgpuDev->VirtIo, ResponseMap);
+ if (EFI_ERROR (Status)) {
+ goto UnmapRequest;
+ }
+ Status = VgpuDev->VirtIo->UnmapSharedBuffer (VgpuDev->VirtIo, RequestMap);
+ if (EFI_ERROR (Status)) {
+ return Status;
+ }
+
+ //
+ // Parse the response.
+ //
if (Response.Type == VirtioGpuRespOkNodata) {
return EFI_SUCCESS;
}
DEBUG ((EFI_D_ERROR, "%a: Request=0x%x Response=0x%x\n", __FUNCTION__,
(UINT32)RequestType, Response.Type));
return EFI_DEVICE_ERROR;
+
+UnmapResponse:
+ VgpuDev->VirtIo->UnmapSharedBuffer (VgpuDev->VirtIo, ResponseMap);
+
+UnmapRequest:
+ VgpuDev->VirtIo->UnmapSharedBuffer (VgpuDev->VirtIo, RequestMap);
+
+ return Status;
}
/**
The following functions send requests to the VirtIo GPU device model, await
the answer from the host, and return a status. They share the following
interface details:
@param[in,out] VgpuDev The VGPU_DEV object that represents the VirtIo GPU
device. The caller is responsible to have
successfully invoked VirtioGpuInit() on VgpuDev
previously, while VirtioGpuUninit() must not have
been called on VgpuDev.
@retval EFI_INVALID_PARAMETER Invalid command-specific parameters were
detected by this driver.
@retval EFI_SUCCESS Operation successful.
@retval EFI_DEVICE_ERROR The host rejected the request. The host error
code has been logged on the EFI_D_ERROR level.
@return Codes for unexpected errors in VirtIo
messaging.
For the command-specific parameters, please consult the GPU Device section of
the VirtIo 1.0 specification (see references in
"OvmfPkg/Include/IndustryStandard/VirtioGpu.h").
**/
--
2.14.1.3.gb7cf6e02401b
^ permalink raw reply related [flat|nested] 11+ messages in thread
* [PATCH 3/6] OvmfPkg/VirtioGpuDxe: take EFI_PHYSICAL_ADDRESS in ResourceAttachBacking()
2017-08-28 13:24 [PATCH 0/6] OvmfPkg/VirtioGpuDxe: map system memory addresses to device addresses Laszlo Ersek
2017-08-28 13:24 ` [PATCH 1/6] OvmfPkg/VirtioGpuDxe: map VRING for bus master common buffer operation Laszlo Ersek
2017-08-28 13:24 ` [PATCH 2/6] OvmfPkg/VirtioGpuDxe: map virtio GPU command objects to device addresses Laszlo Ersek
@ 2017-08-28 13:24 ` Laszlo Ersek
2017-08-28 13:24 ` [PATCH 4/6] OvmfPkg/VirtioGpuDxe: helpers for backing store (de)allocation+(un)mapping Laszlo Ersek
` (3 subsequent siblings)
6 siblings, 0 replies; 11+ messages in thread
From: Laszlo Ersek @ 2017-08-28 13:24 UTC (permalink / raw)
To: edk2-devel-01; +Cc: Ard Biesheuvel, Brijesh Singh, Jordan Justen, Tom Lendacky
The RESOURCE_ATTACH_BACKING virtio GPU command assigns guest-side backing
pages to a host-side resource that was created earlier with the
RESOURCE_CREATE_2D command.
We compose the RESOURCE_ATTACH_BACKING command in the
VirtioGpuResourceAttachBacking() function. Currently this function takes
the parameter
IN VOID *FirstBackingPage
This is only appropriate as long as we pass a (guest-phys) system memory
address to the device. In preparation for a mapped bus master device
address, change the above parameter to
IN EFI_PHYSICAL_ADDRESS BackingStoreDeviceAddress
In order to keep the current call site functional, move the (VOID*) to
(UINTN) conversion out of the function, to the call site.
The "Request.Entry.Addr" field already has type UINT64.
This patch is similar to commit 4b725858de68 ("OvmfPkg/VirtioLib: change
the parameter of VirtioAppendDesc() to UINT64", 2017-08-23).
Cc: Ard Biesheuvel <ard.biesheuvel@linaro.org>
Cc: Brijesh Singh <brijesh.singh@amd.com>
Cc: Jordan Justen <jordan.l.justen@intel.com>
Cc: Tom Lendacky <thomas.lendacky@amd.com>
Contributed-under: TianoCore Contribution Agreement 1.1
Signed-off-by: Laszlo Ersek <lersek@redhat.com>
---
OvmfPkg/VirtioGpuDxe/VirtioGpu.h | 8 ++++----
OvmfPkg/VirtioGpuDxe/Commands.c | 10 +++++-----
OvmfPkg/VirtioGpuDxe/Gop.c | 8 ++++----
3 files changed, 13 insertions(+), 13 deletions(-)
diff --git a/OvmfPkg/VirtioGpuDxe/VirtioGpu.h b/OvmfPkg/VirtioGpuDxe/VirtioGpu.h
index 193e932e1430..cf2a63accd72 100644
--- a/OvmfPkg/VirtioGpuDxe/VirtioGpu.h
+++ b/OvmfPkg/VirtioGpuDxe/VirtioGpu.h
@@ -252,10 +252,10 @@ VirtioGpuResourceUnref (
EFI_STATUS
VirtioGpuResourceAttachBacking (
- IN OUT VGPU_DEV *VgpuDev,
- IN UINT32 ResourceId,
- IN VOID *FirstBackingPage,
- IN UINTN NumberOfPages
+ IN OUT VGPU_DEV *VgpuDev,
+ IN UINT32 ResourceId,
+ IN EFI_PHYSICAL_ADDRESS BackingStoreDeviceAddress,
+ IN UINTN NumberOfPages
);
EFI_STATUS
diff --git a/OvmfPkg/VirtioGpuDxe/Commands.c b/OvmfPkg/VirtioGpuDxe/Commands.c
index bdedea1df6a7..c1951a807e98 100644
--- a/OvmfPkg/VirtioGpuDxe/Commands.c
+++ b/OvmfPkg/VirtioGpuDxe/Commands.c
@@ -496,29 +496,29 @@ VirtioGpuResourceUnref (
EFI_STATUS
VirtioGpuResourceAttachBacking (
- IN OUT VGPU_DEV *VgpuDev,
- IN UINT32 ResourceId,
- IN VOID *FirstBackingPage,
- IN UINTN NumberOfPages
+ IN OUT VGPU_DEV *VgpuDev,
+ IN UINT32 ResourceId,
+ IN EFI_PHYSICAL_ADDRESS BackingStoreDeviceAddress,
+ IN UINTN NumberOfPages
)
{
volatile VIRTIO_GPU_RESOURCE_ATTACH_BACKING Request;
if (ResourceId == 0) {
return EFI_INVALID_PARAMETER;
}
Request.ResourceId = ResourceId;
Request.NrEntries = 1;
- Request.Entry.Addr = (UINTN)FirstBackingPage;
+ Request.Entry.Addr = BackingStoreDeviceAddress;
Request.Entry.Length = (UINT32)EFI_PAGES_TO_SIZE (NumberOfPages);
Request.Entry.Padding = 0;
return VirtioGpuSendCommand (
VgpuDev,
VirtioGpuCmdResourceAttachBacking,
FALSE, // Fence
&Request.Header,
sizeof Request
);
}
diff --git a/OvmfPkg/VirtioGpuDxe/Gop.c b/OvmfPkg/VirtioGpuDxe/Gop.c
index 3438bd03224e..b3c5dae74d0e 100644
--- a/OvmfPkg/VirtioGpuDxe/Gop.c
+++ b/OvmfPkg/VirtioGpuDxe/Gop.c
@@ -229,176 +229,176 @@ EFIAPI
GopSetMode (
IN EFI_GRAPHICS_OUTPUT_PROTOCOL *This,
IN UINT32 ModeNumber
)
{
VGPU_GOP *VgpuGop;
UINT32 NewResourceId;
UINTN NewNumberOfBytes;
UINTN NewNumberOfPages;
VOID *NewBackingStore;
EFI_STATUS Status;
EFI_STATUS Status2;
if (ModeNumber >= ARRAY_SIZE (mGopResolutions)) {
return EFI_UNSUPPORTED;
}
VgpuGop = VGPU_GOP_FROM_GOP (This);
//
// Distinguish the first (internal) call from the other (protocol consumer)
// calls.
//
if (VgpuGop->ResourceId == 0) {
//
// Set up the Gop -> GopMode -> GopModeInfo pointer chain, and the other
// (nonzero) constant fields.
//
// No direct framebuffer access is supported, only Blt() is.
//
VgpuGop->Gop.Mode = &VgpuGop->GopMode;
VgpuGop->GopMode.MaxMode = (UINT32)(ARRAY_SIZE (mGopResolutions));
VgpuGop->GopMode.Info = &VgpuGop->GopModeInfo;
VgpuGop->GopMode.SizeOfInfo = sizeof VgpuGop->GopModeInfo;
VgpuGop->GopModeInfo.PixelFormat = PixelBltOnly;
//
// This is the first time we create a host side resource.
//
NewResourceId = 1;
} else {
//
// We already have an active host side resource. Create the new one without
// interfering with the current one, so that we can cleanly bail out on
// error, without disturbing the current graphics mode.
//
// The formula below will alternate between IDs 1 and 2.
//
NewResourceId = 3 - VgpuGop->ResourceId;
}
//
// Create the 2D host resource.
//
Status = VirtioGpuResourceCreate2d (
VgpuGop->ParentBus, // VgpuDev
NewResourceId, // ResourceId
VirtioGpuFormatB8G8R8X8Unorm, // Format
mGopResolutions[ModeNumber].Width, // Width
mGopResolutions[ModeNumber].Height // Height
);
if (EFI_ERROR (Status)) {
return Status;
}
//
// Allocate guest backing store.
//
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;
goto DestroyHostResource;
}
//
// Fill visible part of backing store with black.
//
ZeroMem (NewBackingStore, NewNumberOfBytes);
//
// Attach backing store to the host resource.
//
Status = VirtioGpuResourceAttachBacking (
- VgpuGop->ParentBus, // VgpuDev
- NewResourceId, // ResourceId
- NewBackingStore, // FirstBackingPage
- NewNumberOfPages // NumberOfPages
+ VgpuGop->ParentBus, // VgpuDev
+ NewResourceId, // ResourceId
+ (UINTN)NewBackingStore, // BackingStoreDeviceAddress
+ NewNumberOfPages // NumberOfPages
);
if (EFI_ERROR (Status)) {
goto FreeBackingStore;
}
//
// Point head (scanout) #0 to the host resource.
//
Status = VirtioGpuSetScanout (
VgpuGop->ParentBus, // VgpuDev
0, // X
0, // Y
mGopResolutions[ModeNumber].Width, // Width
mGopResolutions[ModeNumber].Height, // Height
0, // ScanoutId
NewResourceId // ResourceId
);
if (EFI_ERROR (Status)) {
goto DetachBackingStore;
}
//
// If this is not the first (i.e., internal) call, then we have to (a) flush
// the new resource to head (scanout) #0, after having flipped the latter to
// the former above, plus (b) release the old resources.
//
if (VgpuGop->ResourceId != 0) {
Status = VirtioGpuResourceFlush (
VgpuGop->ParentBus, // VgpuDev
0, // X
0, // Y
mGopResolutions[ModeNumber].Width, // Width
mGopResolutions[ModeNumber].Height, // Height
NewResourceId // ResourceId
);
if (EFI_ERROR (Status)) {
//
// Flip head (scanout) #0 back to the current resource. If this fails, we
// cannot continue, as this error occurs on the error path and is
// therefore non-recoverable.
//
Status2 = VirtioGpuSetScanout (
VgpuGop->ParentBus, // VgpuDev
0, // X
0, // Y
mGopResolutions[This->Mode->Mode].Width, // Width
mGopResolutions[This->Mode->Mode].Height, // Height
0, // ScanoutId
VgpuGop->ResourceId // ResourceId
);
ASSERT_EFI_ERROR (Status2);
if (EFI_ERROR (Status2)) {
CpuDeadLoop ();
}
goto DetachBackingStore;
}
//
// Flush successful; release the old resources (without disabling head
// (scanout) #0).
//
ReleaseGopResources (VgpuGop, FALSE /* DisableHead */);
}
//
// This is either the first (internal) call when we have no old resources
// yet, or we've changed the mode successfully and released the old
// resources.
//
ASSERT (VgpuGop->ResourceId == 0);
ASSERT (VgpuGop->BackingStore == NULL);
VgpuGop->ResourceId = NewResourceId;
VgpuGop->BackingStore = NewBackingStore;
VgpuGop->NumberOfPages = NewNumberOfPages;
//
// Populate Mode and ModeInfo (mutable fields only).
//
VgpuGop->GopMode.Mode = ModeNumber;
VgpuGop->GopModeInfo.HorizontalResolution =
mGopResolutions[ModeNumber].Width;
VgpuGop->GopModeInfo.VerticalResolution = mGopResolutions[ModeNumber].Height;
VgpuGop->GopModeInfo.PixelsPerScanLine = mGopResolutions[ModeNumber].Width;
return EFI_SUCCESS;
--
2.14.1.3.gb7cf6e02401b
^ permalink raw reply related [flat|nested] 11+ messages in thread
* [PATCH 4/6] OvmfPkg/VirtioGpuDxe: helpers for backing store (de)allocation+(un)mapping
2017-08-28 13:24 [PATCH 0/6] OvmfPkg/VirtioGpuDxe: map system memory addresses to device addresses Laszlo Ersek
` (2 preceding siblings ...)
2017-08-28 13:24 ` [PATCH 3/6] OvmfPkg/VirtioGpuDxe: take EFI_PHYSICAL_ADDRESS in ResourceAttachBacking() Laszlo Ersek
@ 2017-08-28 13:24 ` Laszlo Ersek
2017-08-28 13:24 ` [PATCH 5/6] OvmfPkg/VirtioGpuDxe: map backing store to bus master device address Laszlo Ersek
` (2 subsequent siblings)
6 siblings, 0 replies; 11+ messages in thread
From: Laszlo Ersek @ 2017-08-28 13:24 UTC (permalink / raw)
To: edk2-devel-01; +Cc: Ard Biesheuvel, Brijesh Singh, Jordan Justen, Tom Lendacky
Introduce the VirtioGpuAllocateZeroAndMapBackingStore() and
VirtioGpuUnmapAndFreeBackingStore() helper functions. These functions tie
together the allocation, zeroing and mapping, and unmapping and
deallocation, respectively, of memory that the virtio GPU will permanently
reference after receiving the RESOURCE_ATTACH_BACKING command.
With these functions we can keep the next patch simpler -- the GOP
implementation in "Gop.c" retains its error handling structure, and
remains oblivious to VIRTIO_DEVICE_PROTOCOL and VirtioLib.
Cc: Ard Biesheuvel <ard.biesheuvel@linaro.org>
Cc: Brijesh Singh <brijesh.singh@amd.com>
Cc: Jordan Justen <jordan.l.justen@intel.com>
Cc: Tom Lendacky <thomas.lendacky@amd.com>
Contributed-under: TianoCore Contribution Agreement 1.1
Signed-off-by: Laszlo Ersek <lersek@redhat.com>
---
OvmfPkg/VirtioGpuDxe/VirtioGpu.h | 68 +++++++++++
OvmfPkg/VirtioGpuDxe/Commands.c | 120 ++++++++++++++++++++
OvmfPkg/VirtioGpuDxe/DriverBinding.c | 1 -
OvmfPkg/VirtioGpuDxe/Gop.c | 1 -
4 files changed, 188 insertions(+), 2 deletions(-)
diff --git a/OvmfPkg/VirtioGpuDxe/VirtioGpu.h b/OvmfPkg/VirtioGpuDxe/VirtioGpu.h
index cf2a63accd72..65b1bd6088b8 100644
--- a/OvmfPkg/VirtioGpuDxe/VirtioGpu.h
+++ b/OvmfPkg/VirtioGpuDxe/VirtioGpu.h
@@ -1,28 +1,29 @@
/** @file
Internal type and macro definitions for the Virtio GPU hybrid driver.
Copyright (C) 2016, Red Hat, Inc.
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 _VIRTIO_GPU_DXE_H_
#define _VIRTIO_GPU_DXE_H_
#include <IndustryStandard/VirtioGpu.h>
+#include <Library/BaseMemoryLib.h>
#include <Library/DebugLib.h>
#include <Library/UefiLib.h>
#include <Protocol/GraphicsOutput.h>
#include <Protocol/VirtioDevice.h>
//
// Forward declaration of VGPU_GOP.
//
@@ -188,17 +189,84 @@ VOID
VirtioGpuUninit (
IN OUT VGPU_DEV *VgpuDev
);
+/**
+ Allocate, zero and map memory, for bus master common buffer operation, to be
+ attached as backing store to a host-side VirtIo GPU resource.
+
+ @param[in] VgpuDev The VGPU_DEV object that represents the VirtIo GPU
+ device.
+
+ @param[in] NumberOfPages The number of whole pages to allocate and map.
+
+ @param[out] HostAddress The system memory address of the allocated area.
+
+ @param[out] DeviceAddress The bus master device address of the allocated
+ area. The VirtIo GPU device may be programmed to
+ access the allocated area through DeviceAddress;
+ DeviceAddress is to be passed to the
+ VirtioGpuResourceAttachBacking() function, as the
+ BackingStoreDeviceAddress parameter.
+
+ @param[out] Mapping A resulting token to pass to
+ VirtioGpuUnmapAndFreeBackingStore().
+
+ @retval EFI_SUCCESS The requested number of pages has been allocated, zeroed
+ and mapped.
+
+ @return Status codes propagated from
+ VgpuDev->VirtIo->AllocateSharedPages() and
+ VirtioMapAllBytesInSharedBuffer().
+**/
+EFI_STATUS
+VirtioGpuAllocateZeroAndMapBackingStore (
+ IN VGPU_DEV *VgpuDev,
+ IN UINTN NumberOfPages,
+ OUT VOID **HostAddress,
+ OUT EFI_PHYSICAL_ADDRESS *DeviceAddress,
+ OUT VOID **Mapping
+ );
+
+/**
+ Unmap and free memory originally allocated and mapped with
+ VirtioGpuAllocateZeroAndMapBackingStore().
+
+ If the memory allocated and mapped with
+ VirtioGpuAllocateZeroAndMapBackingStore() was attached to a host-side VirtIo
+ GPU resource with VirtioGpuResourceAttachBacking(), then the caller is
+ responsible for detaching the backing store from the same resource, with
+ VirtioGpuResourceDetachBacking(), before calling this function.
+
+ @param[in] VgpuDev The VGPU_DEV object that represents the VirtIo GPU
+ device.
+
+ @param[in] NumberOfPages The NumberOfPages parameter originally passed to
+ VirtioGpuAllocateZeroAndMapBackingStore().
+
+ @param[in] HostAddress The HostAddress value originally output by
+ VirtioGpuAllocateZeroAndMapBackingStore().
+
+ @param[in] Mapping The token that was originally output by
+ VirtioGpuAllocateZeroAndMapBackingStore().
+**/
+VOID
+VirtioGpuUnmapAndFreeBackingStore (
+ IN VGPU_DEV *VgpuDev,
+ IN UINTN NumberOfPages,
+ IN VOID *HostAddress,
+ IN VOID *Mapping
+ );
+
/**
EFI_EVENT_NOTIFY function for the VGPU_DEV.ExitBoot event. It resets the
VirtIo device, causing it to release its resources and to forget its
configuration.
This function may only be called (that is, VGPU_DEV.ExitBoot may only be
signaled) after VirtioGpuInit() returns and before VirtioGpuUninit() is
called.
@param[in] Event Event whose notification function is being invoked.
@param[in] Context Pointer to the associated VGPU_DEV object.
**/
diff --git a/OvmfPkg/VirtioGpuDxe/Commands.c b/OvmfPkg/VirtioGpuDxe/Commands.c
index c1951a807e98..595a3717d926 100644
--- a/OvmfPkg/VirtioGpuDxe/Commands.c
+++ b/OvmfPkg/VirtioGpuDxe/Commands.c
@@ -201,26 +201,146 @@ VOID
VirtioGpuUninit (
IN OUT VGPU_DEV *VgpuDev
)
{
//
// Resetting the VirtIo device makes it release its resources and forget its
// configuration.
//
VgpuDev->VirtIo->SetDeviceStatus (VgpuDev->VirtIo, 0);
VgpuDev->VirtIo->UnmapSharedBuffer (VgpuDev->VirtIo, VgpuDev->RingMap);
VirtioRingUninit (VgpuDev->VirtIo, &VgpuDev->Ring);
}
+/**
+ Allocate, zero and map memory, for bus master common buffer operation, to be
+ attached as backing store to a host-side VirtIo GPU resource.
+
+ @param[in] VgpuDev The VGPU_DEV object that represents the VirtIo GPU
+ device.
+
+ @param[in] NumberOfPages The number of whole pages to allocate and map.
+
+ @param[out] HostAddress The system memory address of the allocated area.
+
+ @param[out] DeviceAddress The bus master device address of the allocated
+ area. The VirtIo GPU device may be programmed to
+ access the allocated area through DeviceAddress;
+ DeviceAddress is to be passed to the
+ VirtioGpuResourceAttachBacking() function, as the
+ BackingStoreDeviceAddress parameter.
+
+ @param[out] Mapping A resulting token to pass to
+ VirtioGpuUnmapAndFreeBackingStore().
+
+ @retval EFI_SUCCESS The requested number of pages has been allocated, zeroed
+ and mapped.
+
+ @return Status codes propagated from
+ VgpuDev->VirtIo->AllocateSharedPages() and
+ VirtioMapAllBytesInSharedBuffer().
+**/
+EFI_STATUS
+VirtioGpuAllocateZeroAndMapBackingStore (
+ IN VGPU_DEV *VgpuDev,
+ IN UINTN NumberOfPages,
+ OUT VOID **HostAddress,
+ OUT EFI_PHYSICAL_ADDRESS *DeviceAddress,
+ OUT VOID **Mapping
+ )
+{
+ EFI_STATUS Status;
+ VOID *NewHostAddress;
+
+ Status = VgpuDev->VirtIo->AllocateSharedPages (
+ VgpuDev->VirtIo,
+ NumberOfPages,
+ &NewHostAddress
+ );
+ if (EFI_ERROR (Status)) {
+ return Status;
+ }
+
+ //
+ // Avoid exposing stale data to the device even temporarily: zero the area
+ // before mapping it.
+ //
+ ZeroMem (NewHostAddress, EFI_PAGES_TO_SIZE (NumberOfPages));
+
+ Status = VirtioMapAllBytesInSharedBuffer (
+ VgpuDev->VirtIo, // VirtIo
+ VirtioOperationBusMasterCommonBuffer, // Operation
+ NewHostAddress, // HostAddress
+ EFI_PAGES_TO_SIZE (NumberOfPages), // NumberOfBytes
+ DeviceAddress, // DeviceAddress
+ Mapping // Mapping
+ );
+ if (EFI_ERROR (Status)) {
+ goto FreeSharedPages;
+ }
+
+ *HostAddress = NewHostAddress;
+ return EFI_SUCCESS;
+
+FreeSharedPages:
+ VgpuDev->VirtIo->FreeSharedPages (
+ VgpuDev->VirtIo,
+ NumberOfPages,
+ NewHostAddress
+ );
+ return Status;
+}
+
+/**
+ Unmap and free memory originally allocated and mapped with
+ VirtioGpuAllocateZeroAndMapBackingStore().
+
+ If the memory allocated and mapped with
+ VirtioGpuAllocateZeroAndMapBackingStore() was attached to a host-side VirtIo
+ GPU resource with VirtioGpuResourceAttachBacking(), then the caller is
+ responsible for detaching the backing store from the same resource, with
+ VirtioGpuResourceDetachBacking(), before calling this function.
+
+ @param[in] VgpuDev The VGPU_DEV object that represents the VirtIo GPU
+ device.
+
+ @param[in] NumberOfPages The NumberOfPages parameter originally passed to
+ VirtioGpuAllocateZeroAndMapBackingStore().
+
+ @param[in] HostAddress The HostAddress value originally output by
+ VirtioGpuAllocateZeroAndMapBackingStore().
+
+ @param[in] Mapping The token that was originally output by
+ VirtioGpuAllocateZeroAndMapBackingStore().
+**/
+VOID
+VirtioGpuUnmapAndFreeBackingStore (
+ IN VGPU_DEV *VgpuDev,
+ IN UINTN NumberOfPages,
+ IN VOID *HostAddress,
+ IN VOID *Mapping
+ )
+{
+ VgpuDev->VirtIo->UnmapSharedBuffer (
+ VgpuDev->VirtIo,
+ Mapping
+ );
+ VgpuDev->VirtIo->FreeSharedPages (
+ VgpuDev->VirtIo,
+ NumberOfPages,
+ HostAddress
+ );
+}
+
/**
EFI_EVENT_NOTIFY function for the VGPU_DEV.ExitBoot event. It resets the
VirtIo device, causing it to release its resources and to forget its
configuration.
This function may only be called (that is, VGPU_DEV.ExitBoot may only be
signaled) after VirtioGpuInit() returns and before VirtioGpuUninit() is
called.
@param[in] Event Event whose notification function is being invoked.
@param[in] Context Pointer to the associated VGPU_DEV object.
**/
diff --git a/OvmfPkg/VirtioGpuDxe/DriverBinding.c b/OvmfPkg/VirtioGpuDxe/DriverBinding.c
index 33c1ad3b3110..a44d52cc810b 100644
--- a/OvmfPkg/VirtioGpuDxe/DriverBinding.c
+++ b/OvmfPkg/VirtioGpuDxe/DriverBinding.c
@@ -1,38 +1,37 @@
/** @file
Implement the Driver Binding Protocol and the Component Name 2 Protocol for
the Virtio GPU hybrid driver.
Copyright (C) 2016, Red Hat, Inc.
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/BaseMemoryLib.h>
#include <Library/DevicePathLib.h>
#include <Library/MemoryAllocationLib.h>
#include <Library/PrintLib.h>
#include <Library/UefiBootServicesTableLib.h>
#include <Library/UefiLib.h>
#include <Protocol/ComponentName2.h>
#include <Protocol/DevicePath.h>
#include <Protocol/DriverBinding.h>
#include <Protocol/PciIo.h>
#include "VirtioGpu.h"
//
// The device path node that describes the Video Output Device Attributes for
// the single head (UEFI child handle) that we support.
//
// The ACPI_DISPLAY_ADR() macro corresponds to Table B-2, section "B.4.2 _DOD"
// in the ACPI 3.0b spec, or more recently, to Table B-379, section "B.3.2
// _DOD" in the ACPI 6.0 spec.
//
diff --git a/OvmfPkg/VirtioGpuDxe/Gop.c b/OvmfPkg/VirtioGpuDxe/Gop.c
index b3c5dae74d0e..507e1a770d10 100644
--- a/OvmfPkg/VirtioGpuDxe/Gop.c
+++ b/OvmfPkg/VirtioGpuDxe/Gop.c
@@ -1,44 +1,43 @@
/** @file
EFI_GRAPHICS_OUTPUT_PROTOCOL member functions for the VirtIo GPU driver.
Copyright (C) 2016, Red Hat, Inc.
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/BaseMemoryLib.h>
#include <Library/MemoryAllocationLib.h>
#include "VirtioGpu.h"
/**
Release guest-side and host-side resources that are related to an initialized
VGPU_GOP.Gop.
param[in,out] VgpuGop The VGPU_GOP object to release resources for.
On input, the caller is responsible for having called
VgpuGop->Gop.SetMode() at least once successfully.
(This is equivalent to the requirement that
VgpuGop->BackingStore be non-NULL. It is also
equivalent to the requirement that VgpuGop->ResourceId
be nonzero.)
On output, resources will be released, and
VgpuGop->BackingStore and VgpuGop->ResourceId will be
nulled.
param[in] DisableHead Whether this head (scanout) currently references the
resource identified by VgpuGop->ResourceId. Only pass
FALSE when VgpuGop->Gop.SetMode() calls this function
while switching between modes, and set it to TRUE
every other time.
**/
--
2.14.1.3.gb7cf6e02401b
^ permalink raw reply related [flat|nested] 11+ messages in thread
* [PATCH 5/6] OvmfPkg/VirtioGpuDxe: map backing store to bus master device address
2017-08-28 13:24 [PATCH 0/6] OvmfPkg/VirtioGpuDxe: map system memory addresses to device addresses Laszlo Ersek
` (3 preceding siblings ...)
2017-08-28 13:24 ` [PATCH 4/6] OvmfPkg/VirtioGpuDxe: helpers for backing store (de)allocation+(un)mapping Laszlo Ersek
@ 2017-08-28 13:24 ` Laszlo Ersek
2017-08-28 13:24 ` [PATCH 6/6] OvmfPkg/VirtioGpuDxe: negotiate VIRTIO_F_IOMMU_PLATFORM Laszlo Ersek
2017-08-29 23:02 ` [PATCH 0/6] OvmfPkg/VirtioGpuDxe: map system memory addresses to device addresses Laszlo Ersek
6 siblings, 0 replies; 11+ messages in thread
From: Laszlo Ersek @ 2017-08-28 13:24 UTC (permalink / raw)
To: edk2-devel-01; +Cc: Ard Biesheuvel, Brijesh Singh, Jordan Justen, Tom Lendacky
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 <ard.biesheuvel@linaro.org>
Cc: Brijesh Singh <brijesh.singh@amd.com>
Cc: Jordan Justen <jordan.l.justen@intel.com>
Cc: Tom Lendacky <thomas.lendacky@amd.com>
Contributed-under: TianoCore Contribution Agreement 1.1
Signed-off-by: Laszlo Ersek <lersek@redhat.com>
---
OvmfPkg/VirtioGpuDxe/VirtioGpu.h | 7 +++
OvmfPkg/VirtioGpuDxe/Commands.c | 21 ++++++++
OvmfPkg/VirtioGpuDxe/Gop.c | 55 +++++++++++++-------
3 files changed, 64 insertions(+), 19 deletions(-)
diff --git a/OvmfPkg/VirtioGpuDxe/VirtioGpu.h b/OvmfPkg/VirtioGpuDxe/VirtioGpu.h
index 65b1bd6088b8..73893ccb56eb 100644
--- a/OvmfPkg/VirtioGpuDxe/VirtioGpu.h
+++ b/OvmfPkg/VirtioGpuDxe/VirtioGpu.h
@@ -94,80 +94,87 @@ typedef struct {
struct VGPU_GOP_STRUCT {
UINT64 Signature;
//
// ParentBus points to the parent VGPU_DEV object. Never NULL.
//
VGPU_DEV *ParentBus;
//
// GopName carries a customized name for
// EFI_COMPONENT_NAME2_PROTOCOL.GetControllerName(). It is expressed in table
// form because it can theoretically support several languages. Never NULL.
//
EFI_UNICODE_STRING_TABLE *GopName;
//
// GopHandle is the UEFI child handle that carries the device path ending
// with the ACPI ADR node, and the Graphics Output Protocol. Never NULL.
//
EFI_HANDLE GopHandle;
//
// The GopDevicePath field is the device path installed on GopHandle,
// ending with an ACPI ADR node. Never NULL.
//
EFI_DEVICE_PATH_PROTOCOL *GopDevicePath;
//
// The Gop field is installed on the child handle as Graphics Output Protocol
// interface.
//
EFI_GRAPHICS_OUTPUT_PROTOCOL Gop;
//
// Referenced by Gop.Mode, GopMode provides a summary about the supported
// graphics modes, and the current mode.
//
EFI_GRAPHICS_OUTPUT_PROTOCOL_MODE GopMode;
//
// Referenced by GopMode.Info, GopModeInfo provides detailed information
// about the current mode.
//
EFI_GRAPHICS_OUTPUT_MODE_INFORMATION GopModeInfo;
//
// Identifier of the 2D host resource that is in use by this head (scanout)
// of the VirtIo GPU device. Zero until the first successful -- internal --
// Gop.SetMode() call, never zero afterwards.
//
UINT32 ResourceId;
//
// A number of whole pages providing the backing store for the 2D host
// resource identified by ResourceId above. NULL until the first successful
// -- internal -- Gop.SetMode() call, never NULL afterwards.
//
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;
};
//
// VirtIo GPU initialization, and commands (primitives) for the GPU device.
//
/**
Configure the VirtIo GPU device that underlies VgpuDev.
@param[in,out] VgpuDev The VGPU_DEV object to set up VirtIo messaging for.
On input, the caller is responsible for having
initialized VgpuDev->VirtIo. On output, VgpuDev->Ring
has been initialized, and synchronous VirtIo GPU
commands (primitives) can be submitted to the device.
@retval EFI_SUCCESS VirtIo GPU configuration successful.
@retval EFI_UNSUPPORTED The host-side configuration of the VirtIo GPU is not
supported by this driver.
@retval Error codes from underlying functions.
**/
diff --git a/OvmfPkg/VirtioGpuDxe/Commands.c b/OvmfPkg/VirtioGpuDxe/Commands.c
index 595a3717d926..db5bdbca4bee 100644
--- a/OvmfPkg/VirtioGpuDxe/Commands.c
+++ b/OvmfPkg/VirtioGpuDxe/Commands.c
@@ -349,55 +349,76 @@ EFIAPI
VirtioGpuExitBoot (
IN EFI_EVENT Event,
IN VOID *Context
)
{
VGPU_DEV *VgpuDev;
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);
}
/**
Internal utility function that sends a request to the VirtIo GPU device
model, awaits the answer from the host, and returns a status.
@param[in,out] VgpuDev The VGPU_DEV object that represents the VirtIo GPU
device. The caller is responsible to have
successfully invoked VirtioGpuInit() on VgpuDev
previously, while VirtioGpuUninit() must not have
been called on VgpuDev.
@param[in] RequestType The type of the request. The caller is responsible
for providing a VirtioGpuCmd* RequestType which, on
success, elicits a VirtioGpuRespOkNodata response
from the host.
@param[in] Fence Whether to enable fencing for this request. Fencing
forces the host to complete the command before
producing a response. If Fence is TRUE, then
VgpuDev->FenceId is consumed, and incremented.
@param[in,out] Header Pointer to the caller-allocated request object. The
request must start with VIRTIO_GPU_CONTROL_HEADER.
This function overwrites all fields of Header before
submitting the request to the host:
- it sets Type from RequestType,
- it sets Flags and FenceId based on Fence,
- it zeroes CtxId and Padding.
@param[in] RequestSize Size of the entire caller-allocated request object,
including the leading VIRTIO_GPU_CONTROL_HEADER.
@retval EFI_SUCCESS Operation successful.
@retval EFI_DEVICE_ERROR The host rejected the request. The host error
code has been logged on the EFI_D_ERROR level.
@return Codes for unexpected errors in VirtIo
messaging, or request/response
mapping/unmapping.
**/
diff --git a/OvmfPkg/VirtioGpuDxe/Gop.c b/OvmfPkg/VirtioGpuDxe/Gop.c
index 507e1a770d10..936e181e8a87 100644
--- a/OvmfPkg/VirtioGpuDxe/Gop.c
+++ b/OvmfPkg/VirtioGpuDxe/Gop.c
@@ -45,95 +45,101 @@ VOID
ReleaseGopResources (
IN OUT VGPU_GOP *VgpuGop,
IN BOOLEAN DisableHead
)
{
EFI_STATUS Status;
ASSERT (VgpuGop->ResourceId != 0);
ASSERT (VgpuGop->BackingStore != NULL);
//
// If any of the following host-side destruction steps fail, we can't get out
// of an inconsistent state, so we'll hang. In general errors in object
// destruction can hardly be recovered from.
//
if (DisableHead) {
//
// Dissociate head (scanout) #0 from the currently used 2D host resource,
// by setting ResourceId=0 for it.
//
Status = VirtioGpuSetScanout (
VgpuGop->ParentBus, // VgpuDev
0, 0, 0, 0, // X, Y, Width, Height
0, // ScanoutId
0 // ResourceId
);
//
// HACK BEGINS HERE
//
// According to the GPU Device section of the VirtIo specification, the
// above operation is valid:
//
// "The driver can use resource_id = 0 to disable a scanout."
//
// However, in practice QEMU does not allow us to disable head (scanout) #0
// -- it rejects the command with response code 0x1202
// (VIRTIO_GPU_RESP_ERR_INVALID_SCANOUT_ID). Looking at the QEMU source
// code, function virtio_gpu_set_scanout() in "hw/display/virtio-gpu.c",
// this appears fully intentional, despite not being documented in the
// spec.
//
// Surprisingly, ignoring the error here, and proceeding to release
// host-side resources that presumably underlie head (scanout) #0, work
// without any problems -- the driver survives repeated "disconnect" /
// "connect -r" commands in the UEFI shell.
//
// So, for now, let's just suppress the error.
//
Status = EFI_SUCCESS;
//
// HACK ENDS HERE
//
ASSERT_EFI_ERROR (Status);
if (EFI_ERROR (Status)) {
CpuDeadLoop ();
}
}
//
// Detach backing pages from the currently used 2D host resource.
//
Status = VirtioGpuResourceDetachBacking (
VgpuGop->ParentBus, // VgpuDev
VgpuGop->ResourceId // ResourceId
);
ASSERT_EFI_ERROR (Status);
if (EFI_ERROR (Status)) {
CpuDeadLoop ();
}
//
- // Release backing pages.
+ // Unmap and release backing pages.
//
- FreePages (VgpuGop->BackingStore, VgpuGop->NumberOfPages);
+ 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.
//
Status = VirtioGpuResourceUnref (
VgpuGop->ParentBus, // VgpuDev
VgpuGop->ResourceId // ResourceId
);
ASSERT_EFI_ERROR (Status);
if (EFI_ERROR (Status)) {
CpuDeadLoop ();
}
VgpuGop->ResourceId = 0;
}
//
// The resolutions supported by this driver.
//
@@ -228,176 +234,182 @@ EFIAPI
GopSetMode (
IN EFI_GRAPHICS_OUTPUT_PROTOCOL *This,
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;
if (ModeNumber >= ARRAY_SIZE (mGopResolutions)) {
return EFI_UNSUPPORTED;
}
VgpuGop = VGPU_GOP_FROM_GOP (This);
//
// Distinguish the first (internal) call from the other (protocol consumer)
// calls.
//
if (VgpuGop->ResourceId == 0) {
//
// Set up the Gop -> GopMode -> GopModeInfo pointer chain, and the other
// (nonzero) constant fields.
//
// No direct framebuffer access is supported, only Blt() is.
//
VgpuGop->Gop.Mode = &VgpuGop->GopMode;
VgpuGop->GopMode.MaxMode = (UINT32)(ARRAY_SIZE (mGopResolutions));
VgpuGop->GopMode.Info = &VgpuGop->GopModeInfo;
VgpuGop->GopMode.SizeOfInfo = sizeof VgpuGop->GopModeInfo;
VgpuGop->GopModeInfo.PixelFormat = PixelBltOnly;
//
// This is the first time we create a host side resource.
//
NewResourceId = 1;
} else {
//
// We already have an active host side resource. Create the new one without
// interfering with the current one, so that we can cleanly bail out on
// error, without disturbing the current graphics mode.
//
// The formula below will alternate between IDs 1 and 2.
//
NewResourceId = 3 - VgpuGop->ResourceId;
}
//
// Create the 2D host resource.
//
Status = VirtioGpuResourceCreate2d (
VgpuGop->ParentBus, // VgpuDev
NewResourceId, // ResourceId
VirtioGpuFormatB8G8R8X8Unorm, // Format
mGopResolutions[ModeNumber].Width, // Width
mGopResolutions[ModeNumber].Height // Height
);
if (EFI_ERROR (Status)) {
return Status;
}
//
- // 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.
//
Status = VirtioGpuResourceAttachBacking (
VgpuGop->ParentBus, // VgpuDev
NewResourceId, // ResourceId
- (UINTN)NewBackingStore, // BackingStoreDeviceAddress
+ NewBackingStoreDeviceAddress, // BackingStoreDeviceAddress
NewNumberOfPages // NumberOfPages
);
if (EFI_ERROR (Status)) {
- goto FreeBackingStore;
+ goto UnmapAndFreeBackingStore;
}
//
// Point head (scanout) #0 to the host resource.
//
Status = VirtioGpuSetScanout (
VgpuGop->ParentBus, // VgpuDev
0, // X
0, // Y
mGopResolutions[ModeNumber].Width, // Width
mGopResolutions[ModeNumber].Height, // Height
0, // ScanoutId
NewResourceId // ResourceId
);
if (EFI_ERROR (Status)) {
goto DetachBackingStore;
}
//
// If this is not the first (i.e., internal) call, then we have to (a) flush
// the new resource to head (scanout) #0, after having flipped the latter to
// the former above, plus (b) release the old resources.
//
if (VgpuGop->ResourceId != 0) {
Status = VirtioGpuResourceFlush (
VgpuGop->ParentBus, // VgpuDev
0, // X
0, // Y
mGopResolutions[ModeNumber].Width, // Width
mGopResolutions[ModeNumber].Height, // Height
NewResourceId // ResourceId
);
if (EFI_ERROR (Status)) {
//
// Flip head (scanout) #0 back to the current resource. If this fails, we
// cannot continue, as this error occurs on the error path and is
// therefore non-recoverable.
//
Status2 = VirtioGpuSetScanout (
VgpuGop->ParentBus, // VgpuDev
0, // X
0, // Y
mGopResolutions[This->Mode->Mode].Width, // Width
mGopResolutions[This->Mode->Mode].Height, // Height
0, // ScanoutId
VgpuGop->ResourceId // ResourceId
);
ASSERT_EFI_ERROR (Status2);
if (EFI_ERROR (Status2)) {
CpuDeadLoop ();
}
goto DetachBackingStore;
}
//
// Flush successful; release the old resources (without disabling head
// (scanout) #0).
//
ReleaseGopResources (VgpuGop, FALSE /* DisableHead */);
}
//
// This is either the first (internal) call when we have no old resources
// yet, or we've changed the mode successfully and released the old
// resources.
//
ASSERT (VgpuGop->ResourceId == 0);
ASSERT (VgpuGop->BackingStore == NULL);
VgpuGop->ResourceId = NewResourceId;
VgpuGop->BackingStore = NewBackingStore;
VgpuGop->NumberOfPages = NewNumberOfPages;
+ VgpuGop->BackingStoreMap = NewBackingStoreMap;
//
// Populate Mode and ModeInfo (mutable fields only).
//
VgpuGop->GopMode.Mode = ModeNumber;
VgpuGop->GopModeInfo.HorizontalResolution =
mGopResolutions[ModeNumber].Width;
VgpuGop->GopModeInfo.VerticalResolution = mGopResolutions[ModeNumber].Height;
VgpuGop->GopModeInfo.PixelsPerScanLine = mGopResolutions[ModeNumber].Width;
return EFI_SUCCESS;
@@ -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);
--
2.14.1.3.gb7cf6e02401b
^ permalink raw reply related [flat|nested] 11+ messages in thread
* [PATCH 6/6] OvmfPkg/VirtioGpuDxe: negotiate VIRTIO_F_IOMMU_PLATFORM
2017-08-28 13:24 [PATCH 0/6] OvmfPkg/VirtioGpuDxe: map system memory addresses to device addresses Laszlo Ersek
` (4 preceding siblings ...)
2017-08-28 13:24 ` [PATCH 5/6] OvmfPkg/VirtioGpuDxe: map backing store to bus master device address Laszlo Ersek
@ 2017-08-28 13:24 ` Laszlo Ersek
2017-08-29 23:02 ` [PATCH 0/6] OvmfPkg/VirtioGpuDxe: map system memory addresses to device addresses Laszlo Ersek
6 siblings, 0 replies; 11+ messages in thread
From: Laszlo Ersek @ 2017-08-28 13:24 UTC (permalink / raw)
To: edk2-devel-01; +Cc: Ard Biesheuvel, Brijesh Singh, Jordan Justen, Tom Lendacky
VirtioGpuDxe is now IOMMU-clean; it translates system memory addresses to
bus master device addresses. Negotiate VIRTIO_F_IOMMU_PLATFORM in parallel
with VIRTIO_F_VERSION_1. (Note: the VirtIo GPU device, and this driver,
are virtio-1.0 only (a.k.a. "modern-only").)
Cc: Ard Biesheuvel <ard.biesheuvel@linaro.org>
Cc: Brijesh Singh <brijesh.singh@amd.com>
Cc: Jordan Justen <jordan.l.justen@intel.com>
Cc: Tom Lendacky <thomas.lendacky@amd.com>
Contributed-under: TianoCore Contribution Agreement 1.1
Signed-off-by: Laszlo Ersek <lersek@redhat.com>
---
OvmfPkg/VirtioGpuDxe/Commands.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/OvmfPkg/VirtioGpuDxe/Commands.c b/OvmfPkg/VirtioGpuDxe/Commands.c
index db5bdbca4bee..6e70b1c33f65 100644
--- a/OvmfPkg/VirtioGpuDxe/Commands.c
+++ b/OvmfPkg/VirtioGpuDxe/Commands.c
@@ -39,131 +39,131 @@ EFI_STATUS
VirtioGpuInit (
IN OUT VGPU_DEV *VgpuDev
)
{
UINT8 NextDevStat;
EFI_STATUS Status;
UINT64 Features;
UINT16 QueueSize;
UINT64 RingBaseShift;
//
// Execute virtio-v1.0-cs04, 3.1.1 Driver Requirements: Device
// Initialization.
//
// 1. Reset the device.
//
NextDevStat = 0;
Status = VgpuDev->VirtIo->SetDeviceStatus (VgpuDev->VirtIo, NextDevStat);
if (EFI_ERROR (Status)) {
goto Failed;
}
//
// 2. Set the ACKNOWLEDGE status bit [...]
//
NextDevStat |= VSTAT_ACK;
Status = VgpuDev->VirtIo->SetDeviceStatus (VgpuDev->VirtIo, NextDevStat);
if (EFI_ERROR (Status)) {
goto Failed;
}
//
// 3. Set the DRIVER status bit [...]
//
NextDevStat |= VSTAT_DRIVER;
Status = VgpuDev->VirtIo->SetDeviceStatus (VgpuDev->VirtIo, NextDevStat);
if (EFI_ERROR (Status)) {
goto Failed;
}
//
// 4. Read device feature bits...
//
Status = VgpuDev->VirtIo->GetDeviceFeatures (VgpuDev->VirtIo, &Features);
if (EFI_ERROR (Status)) {
goto Failed;
}
if ((Features & VIRTIO_F_VERSION_1) == 0) {
Status = EFI_UNSUPPORTED;
goto Failed;
}
//
// We only want the most basic 2D features.
//
- Features &= VIRTIO_F_VERSION_1;
+ Features &= VIRTIO_F_VERSION_1 | VIRTIO_F_IOMMU_PLATFORM;
//
// ... and write the subset of feature bits understood by the [...] driver to
// the device. [...]
// 5. Set the FEATURES_OK status bit.
// 6. Re-read device status to ensure the FEATURES_OK bit is still set [...]
//
Status = Virtio10WriteFeatures (VgpuDev->VirtIo, Features, &NextDevStat);
if (EFI_ERROR (Status)) {
goto Failed;
}
//
// 7. Perform device-specific setup, including discovery of virtqueues for
// the device [...]
//
Status = VgpuDev->VirtIo->SetQueueSel (VgpuDev->VirtIo,
VIRTIO_GPU_CONTROL_QUEUE);
if (EFI_ERROR (Status)) {
goto Failed;
}
Status = VgpuDev->VirtIo->GetQueueNumMax (VgpuDev->VirtIo, &QueueSize);
if (EFI_ERROR (Status)) {
goto Failed;
}
//
// We implement each VirtIo GPU command that we use with two descriptors:
// request, response.
//
if (QueueSize < 2) {
Status = EFI_UNSUPPORTED;
goto Failed;
}
//
// [...] population of virtqueues [...]
//
Status = VirtioRingInit (VgpuDev->VirtIo, QueueSize, &VgpuDev->Ring);
if (EFI_ERROR (Status)) {
goto Failed;
}
//
// If anything fails from here on, we have to release the ring.
//
Status = VirtioRingMap (
VgpuDev->VirtIo,
&VgpuDev->Ring,
&RingBaseShift,
&VgpuDev->RingMap
);
if (EFI_ERROR (Status)) {
goto ReleaseQueue;
}
//
// If anything fails from here on, we have to unmap the ring.
//
Status = VgpuDev->VirtIo->SetQueueAddress (
VgpuDev->VirtIo,
&VgpuDev->Ring,
RingBaseShift
);
if (EFI_ERROR (Status)) {
goto UnmapQueue;
}
//
// 8. Set the DRIVER_OK status bit.
//
NextDevStat |= VSTAT_DRIVER_OK;
Status = VgpuDev->VirtIo->SetDeviceStatus (VgpuDev->VirtIo, NextDevStat);
if (EFI_ERROR (Status)) {
goto UnmapQueue;
}
return EFI_SUCCESS;
--
2.14.1.3.gb7cf6e02401b
^ permalink raw reply related [flat|nested] 11+ messages in thread
* Re: [PATCH 0/6] OvmfPkg/VirtioGpuDxe: map system memory addresses to device addresses
2017-08-28 13:24 [PATCH 0/6] OvmfPkg/VirtioGpuDxe: map system memory addresses to device addresses Laszlo Ersek
` (5 preceding siblings ...)
2017-08-28 13:24 ` [PATCH 6/6] OvmfPkg/VirtioGpuDxe: negotiate VIRTIO_F_IOMMU_PLATFORM Laszlo Ersek
@ 2017-08-29 23:02 ` Laszlo Ersek
2017-08-30 15:47 ` Brijesh Singh
6 siblings, 1 reply; 11+ messages in thread
From: Laszlo Ersek @ 2017-08-29 23:02 UTC (permalink / raw)
To: edk2-devel-01; +Cc: Jordan Justen, Tom Lendacky, Ard Biesheuvel, Brijesh Singh
On 08/28/17 15:24, Laszlo Ersek wrote:
> Repo: https://github.com/lersek/edk2.git
> Branch: virtio_gpu_map
>
> This series brings IOMMU support to VirtioGpuDxe. The interesting part
> is patch#5.
>
> I formatted the patches with function context, for easier review.
>
> I regression-tested this series in various environments; IA32, X64,
> AARCH64. My access to a SEV machine is still in the making, so I
> couldn't test the patches on SEV.
>
> Brijesh, if you could test the series for me on SEV, that would be
> great. I also plan to test it on SEV, once my access is established.
I have great test results on SEV. Everything works as expected:
- normal display behavior (setup browser, shell commands, scrolling),
- connect, disconnect, reconnect in the UEFI shell, issued from the
serial console,
- mode changes in the UEFI shell (after reconnect too).
I can see the IOMMU / SEV library debug messages too, and I can
associate them with GPU actions.
I also tested ExitBootServices(). In the log, I see the following six
(+1) consecutive lines (broken up here for easier reading):
> 244888 IoMmuUnmap PlainText 0x7D273000 Crypted 0x7D273000 Pages 0x300 Bytes 0x300000
> 244889 IoMmuDxe:InternalMemEncryptSevSetMemoryEncrypted Set C-bit Cr3 0 Base 7D273000 Length 300000 flush 1
> 244890 IoMmuUnmap PlainText 0x7ED81000 Crypted 0x7ED81000 Pages 0x2 Bytes 0x2000
> 244891 IoMmuDxe:InternalMemEncryptSevSetMemoryEncrypted Set C-bit Cr3 0 Base 7ED81000 Length 2000 flush 1
> 244892 IoMmuUnmap PlainText 0x7F0DA000 Crypted 0x7F0DA000 Pages 0x2 Bytes 0x2000
> 244893 IoMmuDxe:InternalMemEncryptSevSetMemoryEncrypted Set C-bit Cr3 0 Base 7F0DA000 Length 2000 flush 1
> 244894 MpInitChangeApLoopCallback() done!
* The first two lines correspond to unmapping (re-encrypting) the
backing store of the GOP's current video mode (see VirtioGpuExitBoot()
in patch #5). Searching the log backwards for the same address
(0x7D273000), I find it associated with the latest MODE change from
the UEFI shell (when this backing store was allocated and decrypted):
> 188052 IoMmuAllocateBuffer Address 0x7D273000 Pages 0x300
> 188053 IoMmuDxe:InternalMemEncryptSevSetMemoryDecrypted Clear C-bit Cr3 0 Base 7D273000 Length 300000 flush 1
> 188054 IoMmuDxe:SetMemoryEncDec Spliting 2MB page at 7D273000
> 188055 IoMmuMap PlainText 0x7D273000 Crypted 0x7D273000 Pages 0x300 Bytes 0x300000
* The second two lines correspond to unmapping (re-encrypting) the
virtio ring for the virtio-gpu device (see VirtioGpuExitBoot() in
patch #1). Searching the log backwards for the same address
(0x7ED81000), I find it associated with the latest connecting of the
virtio-gpu device, which is when the virtio-ring is allocated and
mapped/decrypted (see VirtioRingMap() in patch #1):
> 158667 InstallProtocolInterface: [VirtioDeviceProtocol] 7F104B20
> 158668 IoMmuAllocateBuffer Address 0x7ED81000 Pages 0x2
> 158669 IoMmuDxe:InternalMemEncryptSevSetMemoryDecrypted Clear C-bit Cr3 0 Base 7ED81000 Length 2000 flush 1
> 158670 IoMmuMap PlainText 0x7ED81000 Crypted 0x7ED81000 Pages 0x2 Bytes 0x2000
> ...
> 158700 InstallProtocolInterface: [EfiGraphicsOutputProtocol] 7F1546C0
> 158701 VirtioGpuDriverBindingStart: produced GOP while binding VirtIo=7F104B20
* The third pair of lines stands for unmapping the virtio ring of the
virtio-blk device that I have also configured for the guest. Searching
the log backwards for the same address (0x7F0DA000), we find
> 7347 IoMmuAllocateBuffer Address 0x7F0DA000 Pages 0x2
> 7348 IoMmuDxe:InternalMemEncryptSevSetMemoryDecrypted Clear C-bit Cr3 0 Base 7F0DA000 Length 2000 flush 1
> 7349 IoMmuMap PlainText 0x7F0DA000 Crypted 0x7F0DA000 Pages 0x2 Bytes 0x2000
> 7350 VirtioBlkInit: LbaSize=0x200[B] NumBlocks=0xC800000[Lba]
> 7351 VirtioBlkInit: FirstAligned=0x0[Lba] PhysBlkSize=0x1[Lba]
> 7352 VirtioBlkInit: OptimalTransferLengthGranularity=0x0[Lba]
> 7353 InstallProtocolInterface: [EfiBlockIoProtocol] 7F0DE310
> 7354 InstallProtocolInterface: [EfiDiskIoProtocol] 7F0DE4A0
* The final line that I quoted ("MpInitChangeApLoopCallback") is just
another (unrelated) ExitBootServices() handler's message.
I'm going to call these results successful.
Thanks,
Laszlo
> Cc: Ard Biesheuvel <ard.biesheuvel@linaro.org>
> Cc: Brijesh Singh <brijesh.singh@amd.com>
> Cc: Jordan Justen <jordan.l.justen@intel.com>
> Cc: Tom Lendacky <thomas.lendacky@amd.com>
>
> Thanks,
> Laszlo
>
> Laszlo Ersek (6):
> OvmfPkg/VirtioGpuDxe: map VRING for bus master common buffer operation
> OvmfPkg/VirtioGpuDxe: map virtio GPU command objects to device
> addresses
> OvmfPkg/VirtioGpuDxe: take EFI_PHYSICAL_ADDRESS in
> ResourceAttachBacking()
> OvmfPkg/VirtioGpuDxe: helpers for backing store
> (de)allocation+(un)mapping
> OvmfPkg/VirtioGpuDxe: map backing store to bus master device address
> OvmfPkg/VirtioGpuDxe: negotiate VIRTIO_F_IOMMU_PLATFORM
>
> OvmfPkg/VirtioGpuDxe/VirtioGpu.h | 89 ++++++-
> OvmfPkg/VirtioGpuDxe/Commands.c | 263 ++++++++++++++++++--
> OvmfPkg/VirtioGpuDxe/DriverBinding.c | 1 -
> OvmfPkg/VirtioGpuDxe/Gop.c | 62 +++--
> 4 files changed, 370 insertions(+), 45 deletions(-)
>
^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: [PATCH 0/6] OvmfPkg/VirtioGpuDxe: map system memory addresses to device addresses
2017-08-29 23:02 ` [PATCH 0/6] OvmfPkg/VirtioGpuDxe: map system memory addresses to device addresses Laszlo Ersek
@ 2017-08-30 15:47 ` Brijesh Singh
2017-08-30 16:15 ` Laszlo Ersek
2017-09-01 12:31 ` Laszlo Ersek
0 siblings, 2 replies; 11+ messages in thread
From: Brijesh Singh @ 2017-08-30 15:47 UTC (permalink / raw)
To: Laszlo Ersek, edk2-devel-01
Cc: brijesh.singh, Jordan Justen, Tom Lendacky, Ard Biesheuvel
On 08/29/2017 06:02 PM, Laszlo Ersek wrote:
[...]
>
> * The third pair of lines stands for unmapping the virtio ring of the
> virtio-blk device that I have also configured for the guest. Searching
> the log backwards for the same address (0x7F0DA000), we find
>
>> 7347 IoMmuAllocateBuffer Address 0x7F0DA000 Pages 0x2
>> 7348 IoMmuDxe:InternalMemEncryptSevSetMemoryDecrypted Clear C-bit Cr3 0 Base 7F0DA000 Length 2000 flush 1
>> 7349 IoMmuMap PlainText 0x7F0DA000 Crypted 0x7F0DA000 Pages 0x2 Bytes 0x2000
>> 7350 VirtioBlkInit: LbaSize=0x200[B] NumBlocks=0xC800000[Lba]
>> 7351 VirtioBlkInit: FirstAligned=0x0[Lba] PhysBlkSize=0x1[Lba]
>> 7352 VirtioBlkInit: OptimalTransferLengthGranularity=0x0[Lba]
>> 7353 InstallProtocolInterface: [EfiBlockIoProtocol] 7F0DE310
>> 7354 InstallProtocolInterface: [EfiDiskIoProtocol] 7F0DE4A0
>
> * The final line that I quoted ("MpInitChangeApLoopCallback") is just
> another (unrelated) ExitBootServices() handler's message.
>
> I'm going to call these results successful.
>
> Thanks,
> Laszlo
>
>> Cc: Ard Biesheuvel <ard.biesheuvel@linaro.org>
>> Cc: Brijesh Singh <brijesh.singh@amd.com>
>> Cc: Jordan Justen <jordan.l.justen@intel.com>
>> Cc: Tom Lendacky <thomas.lendacky@amd.com>
>>
Today I tested your branch on SEV guest and it all worked. thank you for the patch
Tested-By: Brijesh Singh <brijesh.singh@amd.com>
^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: [PATCH 0/6] OvmfPkg/VirtioGpuDxe: map system memory addresses to device addresses
2017-08-30 15:47 ` Brijesh Singh
@ 2017-08-30 16:15 ` Laszlo Ersek
2017-09-01 12:31 ` Laszlo Ersek
1 sibling, 0 replies; 11+ messages in thread
From: Laszlo Ersek @ 2017-08-30 16:15 UTC (permalink / raw)
To: Brijesh Singh, edk2-devel-01; +Cc: Jordan Justen, Tom Lendacky, Ard Biesheuvel
On 08/30/17 17:47, Brijesh Singh wrote:
>
>
> On 08/29/2017 06:02 PM, Laszlo Ersek wrote:
>
> [...]
>>
>> * The third pair of lines stands for unmapping the virtio ring of the
>> virtio-blk device that I have also configured for the guest. Searching
>> the log backwards for the same address (0x7F0DA000), we find
>>
>>> 7347 IoMmuAllocateBuffer Address 0x7F0DA000 Pages 0x2
>>> 7348 IoMmuDxe:InternalMemEncryptSevSetMemoryDecrypted Clear C-bit
>>> Cr3 0 Base 7F0DA000 Length 2000 flush 1
>>> 7349 IoMmuMap PlainText 0x7F0DA000 Crypted 0x7F0DA000 Pages 0x2
>>> Bytes 0x2000
>>> 7350 VirtioBlkInit: LbaSize=0x200[B] NumBlocks=0xC800000[Lba]
>>> 7351 VirtioBlkInit: FirstAligned=0x0[Lba] PhysBlkSize=0x1[Lba]
>>> 7352 VirtioBlkInit: OptimalTransferLengthGranularity=0x0[Lba]
>>> 7353 InstallProtocolInterface: [EfiBlockIoProtocol] 7F0DE310
>>> 7354 InstallProtocolInterface: [EfiDiskIoProtocol] 7F0DE4A0
>>
>> * The final line that I quoted ("MpInitChangeApLoopCallback") is just
>> another (unrelated) ExitBootServices() handler's message.
>>
>> I'm going to call these results successful.
>>
>> Thanks,
>> Laszlo
>>
>>> Cc: Ard Biesheuvel <ard.biesheuvel@linaro.org>
>>> Cc: Brijesh Singh <brijesh.singh@amd.com>
>>> Cc: Jordan Justen <jordan.l.justen@intel.com>
>>> Cc: Tom Lendacky <thomas.lendacky@amd.com>
>>>
>
>
> Today I tested your branch on SEV guest and it all worked. thank you for
> the patch
>
> Tested-By: Brijesh Singh <brijesh.singh@amd.com>
Thank you!
Laszlo
^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: [PATCH 0/6] OvmfPkg/VirtioGpuDxe: map system memory addresses to device addresses
2017-08-30 15:47 ` Brijesh Singh
2017-08-30 16:15 ` Laszlo Ersek
@ 2017-09-01 12:31 ` Laszlo Ersek
1 sibling, 0 replies; 11+ messages in thread
From: Laszlo Ersek @ 2017-09-01 12:31 UTC (permalink / raw)
To: Brijesh Singh, edk2-devel-01; +Cc: Jordan Justen, Ard Biesheuvel, Tom Lendacky
On 08/30/17 17:47, Brijesh Singh wrote:
>
>
> On 08/29/2017 06:02 PM, Laszlo Ersek wrote:
>
> [...]
>>
>> * The third pair of lines stands for unmapping the virtio ring of the
>> virtio-blk device that I have also configured for the guest. Searching
>> the log backwards for the same address (0x7F0DA000), we find
>>
>>> 7347 IoMmuAllocateBuffer Address 0x7F0DA000 Pages 0x2
>>> 7348 IoMmuDxe:InternalMemEncryptSevSetMemoryDecrypted Clear C-bit
>>> Cr3 0 Base 7F0DA000 Length 2000 flush 1
>>> 7349 IoMmuMap PlainText 0x7F0DA000 Crypted 0x7F0DA000 Pages 0x2
>>> Bytes 0x2000
>>> 7350 VirtioBlkInit: LbaSize=0x200[B] NumBlocks=0xC800000[Lba]
>>> 7351 VirtioBlkInit: FirstAligned=0x0[Lba] PhysBlkSize=0x1[Lba]
>>> 7352 VirtioBlkInit: OptimalTransferLengthGranularity=0x0[Lba]
>>> 7353 InstallProtocolInterface: [EfiBlockIoProtocol] 7F0DE310
>>> 7354 InstallProtocolInterface: [EfiDiskIoProtocol] 7F0DE4A0
>>
>> * The final line that I quoted ("MpInitChangeApLoopCallback") is just
>> another (unrelated) ExitBootServices() handler's message.
>>
>> I'm going to call these results successful.
>>
>> Thanks,
>> Laszlo
>>
>>> Cc: Ard Biesheuvel <ard.biesheuvel@linaro.org>
>>> Cc: Brijesh Singh <brijesh.singh@amd.com>
>>> Cc: Jordan Justen <jordan.l.justen@intel.com>
>>> Cc: Tom Lendacky <thomas.lendacky@amd.com>
>>>
>
>
> Today I tested your branch on SEV guest and it all worked. thank you for
> the patch
>
> Tested-By: Brijesh Singh <brijesh.singh@amd.com>
Thanks again, pushed as commit range 1afbb85f8736..db52890926b6.
Laszlo
^ permalink raw reply [flat|nested] 11+ messages in thread
end of thread, other threads:[~2017-09-01 12:29 UTC | newest]
Thread overview: 11+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2017-08-28 13:24 [PATCH 0/6] OvmfPkg/VirtioGpuDxe: map system memory addresses to device addresses Laszlo Ersek
2017-08-28 13:24 ` [PATCH 1/6] OvmfPkg/VirtioGpuDxe: map VRING for bus master common buffer operation Laszlo Ersek
2017-08-28 13:24 ` [PATCH 2/6] OvmfPkg/VirtioGpuDxe: map virtio GPU command objects to device addresses Laszlo Ersek
2017-08-28 13:24 ` [PATCH 3/6] OvmfPkg/VirtioGpuDxe: take EFI_PHYSICAL_ADDRESS in ResourceAttachBacking() Laszlo Ersek
2017-08-28 13:24 ` [PATCH 4/6] OvmfPkg/VirtioGpuDxe: helpers for backing store (de)allocation+(un)mapping Laszlo Ersek
2017-08-28 13:24 ` [PATCH 5/6] OvmfPkg/VirtioGpuDxe: map backing store to bus master device address Laszlo Ersek
2017-08-28 13:24 ` [PATCH 6/6] OvmfPkg/VirtioGpuDxe: negotiate VIRTIO_F_IOMMU_PLATFORM Laszlo Ersek
2017-08-29 23:02 ` [PATCH 0/6] OvmfPkg/VirtioGpuDxe: map system memory addresses to device addresses Laszlo Ersek
2017-08-30 15:47 ` Brijesh Singh
2017-08-30 16:15 ` Laszlo Ersek
2017-09-01 12:31 ` Laszlo Ersek
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox