Laszlo,
One minor issue, I got compilation error with GCC48.
/home/brijesh/codomania/edk2-new/edk2/OvmfPkg/IoMmuDxe/AmdSevIoMmu.c: In
function ‘IoMmuUnmap’:
/home/brijesh/codomania/edk2-new/edk2/OvmfPkg/IoMmuDxe/AmdSevIoMmu.c:408:25:
error: ‘CommonBufferHeader’ may be used uninitialized in this function
[-Werror=maybe-uninitialized]
CommonBufferHeader->StashBuffer,
Looks like we need to initialize CommonBufferHeader = NULL to keep GCC48 happy.
thanks
On 08/02/2017 08:09 PM, Brijesh Singh wrote:
On 8/2/17 7:13 PM, Laszlo Ersek wrote:
(CC Andrew)
On 08/03/17 01:01, Brijesh Singh wrote:
On 8/2/17 4:24 PM, Laszlo Ersek wrote:
[Snip]
At the moment, we have the foll+ // The buffer at MapInfo->CryptedAddress
comes from AllocateBuffer().
//
MapInfo->PlainTextAddress = MapInfo->CryptedAddress;
-
//
- // Therefore no mapping is necessary.
+ // Stash the crypted data.
//
- *DeviceAddress = MapInfo->PlainTextAddress;
- *Mapping = NO_MAPPING;
- FreePool (MapInfo);
- return EFI_SUCCESS;
+ CommonBufferHeader = (COMMON_BUFFER_HEADER *)(
+ (UINTN)MapInfo->CryptedAddress - EFI_PAGE_SIZE
+ );
One question, per spec, is it legal for client to call Map() at some
offset within allocated buffer ?
e.g something like this:
* AllocateBuffer (, 1, &Buffer);
* MapBuffer = Buffer + 10;
* Map (, BusMasterCommonBuffer, MappedBuffer, 10, ..) // Bascially Map
10 bytes from offset 10
The input/output parameter names seem to counter-indicate such use.
Namely, AllocateBuffer() outputs a "HostAddress" param, and Map() takes
a "HostAddress" param. Plus we have sentences like this:
Under PciIo.Map():
... only memory allocated via the AllocateBuffer() interface can be
mapped for this type of operation ...
Under PciIo.AllocateBuffer():
The AllocateBuffer() function allocates pages that are suitable for an
EfiPciOperationBusMasterCommonBuffer or
EfiPciOperationBusMasterCommonBuffer64 mapping. This means that the
buffer allocated by this function must support simultaneous access by
both the processor and a PCI Bus Master. The device address that the
PCI Bus Master uses to access *the* buffer can be retrieved with a
call to Map().
This second passage says *the* buffer. (Emphasis mine above.)
If this is legal then we may need to build MapInfo during
AllocateBuffer() to locate the "StashBuffer".
Right, in that case we'd have to build a list of allocated ranges (an
interval tree of sorts) in AllocateBuffer, and convert any
CommonBuffer[64] Map() call to its containing allocation with a search.
It would be worse than that, actually... The pattern you have raised
could be taken one step further: do one AllocateBuffer(), and several
CommonBuffer[64] Map()s into it :) What should happen if those maps are
distinct? What should happen if they overlap? :) I can't even imagine
what this would mean for SEV.
... There are guide-like sections in the generic description of
EFI_PCI_IO_PROTOCOL; Andrew quoted them earlier:
[email protected]">http://mid.mail-archive.com/[email protected]
DMA Bus Master Common Buffer Operation
======================================
* Call AllocateBuffer() to allocate a common buffer.
* Call Map() for EfiPciIoOperationBusMasterCommonBuffer.
* Program the DMA Bus Master with the DeviceAddress returned by Map().
* The common buffer can now be accessed equally by the processor and
the DMA bus master.
* Call Unmap().
* Call FreeBuffer().
Look at page 854 (printed page number: 784) in UEFI 2.7.
Thus, I don't think the usage you raise is permitted.
Sounds good. I did a quick test on SEV hardware, everything seems to be
working well. I have started my stresstest and report the result tomorrow.
-Brijesh
_______________________________________________
edk2-devel mailing list
[email protected]
https://lists.01.org/mailman/listinfo/edk2-devel
