summaryrefslogtreecommitdiffstats
path: root/OvmfPkg
Commit message (Collapse)AuthorAgeFilesLines
...
* OvmfPkg/BaseMemEncryptSevLib: clean up DEBUG prefixesLaszlo Ersek2017-09-011-7/+7
| | | | | | | | | | | | | | | | | The prefix for the SetMemoryEncDec() DEBUG messages should be "ModuleName:FunctionName: " not "ModuleName:FunctionName " Cc: Brijesh Singh <brijesh.singh@amd.com> Cc: Jordan Justen <jordan.l.justen@intel.com> Contributed-under: TianoCore Contribution Agreement 1.1 Signed-off-by: Laszlo Ersek <lersek@redhat.com> Reviewed-by: Brijesh Singh <brijesh.singh@amd.com> Tested-by: Brijesh Singh <brijesh.singh@amd.com>
* OvmfPkg/BaseMemEncryptSevLib: break DEBUG calls to multiple linesLaszlo Ersek2017-09-011-20/+48
| | | | | | | | | | | | | None of the DEBUG macro invocations in SetMemoryEncDec() fit on a single line. Break them to multiple lines, for (a) conforming to the coding style spec, (b) easier modification in later patches. Cc: Brijesh Singh <brijesh.singh@amd.com> Cc: Jordan Justen <jordan.l.justen@intel.com> Contributed-under: TianoCore Contribution Agreement 1.1 Signed-off-by: Laszlo Ersek <lersek@redhat.com> Reviewed-by: Brijesh Singh <brijesh.singh@amd.com> Tested-by: Brijesh Singh <brijesh.singh@amd.com>
* OvmfPkg/BaseMemEncryptSevLib: unify encrypt/decrypt DEBUG messagesLaszlo Ersek2017-09-011-8/+12
| | | | | | | | | | | | Unify the debug messages between InternalMemEncryptSevSetMemoryEncrypted() and InternalMemEncryptSevSetMemoryDecrypted(). Cc: Brijesh Singh <brijesh.singh@amd.com> Cc: Jordan Justen <jordan.l.justen@intel.com> Contributed-under: TianoCore Contribution Agreement 1.1 Signed-off-by: Laszlo Ersek <lersek@redhat.com> Reviewed-by: Brijesh Singh <brijesh.singh@amd.com> Tested-by: Brijesh Singh <brijesh.singh@amd.com>
* OvmfPkg/VirtioScsiDxe: negotiate VIRTIO_F_IOMMU_PLATFORMBrijesh Singh2017-08-311-2/+3
| | | | | | | | | | | | | | | | | | VirtioScsiDxe driver has been updated to use IOMMU-like member functions from VIRTIO_DEVICE_PROTOCOL to translate the system physical address to device address. We do not need to do anything special when VIRTIO_F_IOMMU_PLATFORM bit is present hence treat it in parallel with VIRTIO_F_VERSION_1. Cc: Ard Biesheuvel <ard.biesheuvel@linaro.org> Cc: Jordan Justen <jordan.l.justen@intel.com> Cc: Tom Lendacky <thomas.lendacky@amd.com> Cc: Laszlo Ersek <lersek@redhat.com> Contributed-under: TianoCore Contribution Agreement 1.1 Signed-off-by: Brijesh Singh <brijesh.singh@amd.com> Reviewed-by: Laszlo Ersek <lersek@redhat.com> Regression-tested-by: Laszlo Ersek <lersek@redhat.com> Tested-by: Laszlo Ersek <lersek@redhat.com>
* OvmfPkg/VirtioScsiDxe: map virtio-scsi request and response buffersBrijesh Singh2017-08-311-16/+205
| | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | When device is behind the IOMMU, driver is require to pass the device address of virtio request, response and any memory referenced by those request/response to the bus master. The patch uses IOMMU-like member functions from VIRTIO_DEVICE_PROTOCOL to map request and response buffers system physical address to the device address. - If the buffer need to be accessed by both the processor and a bus master then map with BusMasterCommonBuffer. - If the buffer need to be accessed for a write operation by a bus master then map with BusMasterWrite. However, after a BusMasterWrite Unmap() failure, error reporting via EFI_EXT_SCSI_PASS_THRU_SCSI_REQUEST_PACKET would be very complex, therefore we map such buffers too with BusMasterCommonBuffer. - If the buffer need to be accessed for a read operation by a bus master then map with BusMasterRead. Cc: Ard Biesheuvel <ard.biesheuvel@linaro.org> Cc: Jordan Justen <jordan.l.justen@intel.com> Cc: Tom Lendacky <thomas.lendacky@amd.com> Cc: Laszlo Ersek <lersek@redhat.com> Contributed-under: TianoCore Contribution Agreement 1.1 Signed-off-by: Brijesh Singh <brijesh.singh@amd.com> [lersek@redhat.com: restore lost sentence/paragraph in commit message] [lersek@redhat.com: reindent/reflow "InDataBuffer" comment block] [lersek@redhat.com: cast arg, not result, of EFI_SIZE_TO_PAGES() to UINTN] Reviewed-by: Laszlo Ersek <lersek@redhat.com> Regression-tested-by: Laszlo Ersek <lersek@redhat.com> Tested-by: Laszlo Ersek <lersek@redhat.com>
* OvmfPkg/VirtioScsiDxe: add helper to create a fake host adapter errorBrijesh Singh2017-08-311-7/+33
| | | | | | | | | | | | | | | | | | | | | When virtio request fails we return EFI_DEVICE_ERROR, as per the spec EFI_EXT_SCSI_PASS_THRU_PROTOCOL.PassThru() member function is required to implement elaborated error reporting. The patch refactors out entire block of the code that creates the host adapter error into a separate helper function (ReportHostAdapterError). Suggested-by: Laszlo Ersek <lersek@redhat.com> Cc: Ard Biesheuvel <ard.biesheuvel@linaro.org> Cc: Jordan Justen <jordan.l.justen@intel.com> Cc: Tom Lendacky <thomas.lendacky@amd.com> Cc: Laszlo Ersek <lersek@redhat.com> Contributed-under: TianoCore Contribution Agreement 1.1 Signed-off-by: Brijesh Singh <brijesh.singh@amd.com> [lersek@redhat.com: fix style & typo in ReportHostAdapterError() comment] Reviewed-by: Laszlo Ersek <lersek@redhat.com> Regression-tested-by: Laszlo Ersek <lersek@redhat.com> Tested-by: Laszlo Ersek <lersek@redhat.com>
* OvmfPkg/VirtioScsiDxe: map VRING using VirtioRingMap()Brijesh Singh2017-08-312-10/+38
| | | | | | | | | | | | | | | | When device is behind the IOMMU then driver need to pass the device address when programing the bus master. The patch uses VirtioRingMap() to map the VRING system physical address to device address. Cc: Ard Biesheuvel <ard.biesheuvel@linaro.org> Cc: Jordan Justen <jordan.l.justen@intel.com> Cc: Tom Lendacky <thomas.lendacky@amd.com> Cc: Laszlo Ersek <lersek@redhat.com> Contributed-under: TianoCore Contribution Agreement 1.1 Signed-off-by: Brijesh Singh <brijesh.singh@amd.com> Reviewed-by: Laszlo Ersek <lersek@redhat.com> Regression-tested-by: Laszlo Ersek <lersek@redhat.com> Tested-by: Laszlo Ersek <lersek@redhat.com>
* OvmfPkg/VirtioBlkDxe: Check the return status of unmap data bufferBrijesh Singh2017-08-301-1/+8
| | | | | | | | | | | | | | | | | | | | | | when "RequestIsWrite" is FALSE -- i.e., the CPU wants data from the device, we map "Buffer" for VirtioOperationBusMasterWrite. In this case, checking the return status of Dev->VirtIo->UnmapSharedBuffer (Dev->VirtIo, BufferMapping); is must. If the unmapping fails, then "Buffer" will not contain the actual data from the device, and we must fail the request with EFI_DEVICE_ERROR. Cc: Ard Biesheuvel <ard.biesheuvel@linaro.org> Cc: Jordan Justen <jordan.l.justen@intel.com> Cc: Tom Lendacky <thomas.lendacky@amd.com> Cc: Laszlo Ersek <lersek@redhat.com> Contributed-under: TianoCore Contribution Agreement 1.1 Signed-off-by: Brijesh Singh <brijesh.singh@amd.com> [lersek@redhat.com: fix typos in subject] Reviewed-by: Laszlo Ersek <lersek@redhat.com> Tested-by: Laszlo Ersek <lersek@redhat.com>
* OvmfPkg/QemuFwCfgDxeLib: SEV: zero FW_CFG_DMA_ACCESS before decrypting itLaszlo Ersek2017-08-291-0/+7
| | | | | | | | | | | | | | | | | | | | | | | | | | | There's a small window between - AllocFwCfgDmaAccessBuffer() mapping the new FW_CFG_DMA_ACCESS object for common buffer operation (i.e., decrypting it), and - InternalQemuFwCfgDmaBytes() setting the fields of the object. In this window, earlier garbage in the object is "leaked" to the hypervisor. So zero the object before we decrypt it. (This commit message references AMD SEV directly, because QemuFwCfgDxeLib is not *generally* enabled for IOMMU operation just yet, unlike our goal for the virtio infrastructure. Instead, QemuFwCfgDxeLib uses MemEncryptSevLib explicitly to detect SEV, and then relies on IOMMU protocol behavior that is specific to SEV. At this point, this is by design.) 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> Reviewed-by: Jordan Justen <jordan.l.justen@intel.com> Reviewed-by: Brijesh Singh <brijesh.singh@amd.com>
* OvmfPkg/VirtioBlkDxe: negotiate VIRTIO_F_IOMMU_PLATFORMBrijesh Singh2017-08-281-2/+3
| | | | | | | | | | | | | | | | | VirtioBlkDxe driver has been updated to use IOMMU-like member functions from VIRTIO_DEVICE_PROTOCOL to translate the system physical address to device address. We do not need to do anything special when VIRTIO_F_IOMMU_PLATFORM bit is present hence treat it in parallel with VIRTIO_F_VERSION_1. Cc: Ard Biesheuvel <ard.biesheuvel@linaro.org> Cc: Jordan Justen <jordan.l.justen@intel.com> Cc: Tom Lendacky <thomas.lendacky@amd.com> Cc: Laszlo Ersek <lersek@redhat.com> Contributed-under: TianoCore Contribution Agreement 1.1 Signed-off-by: Brijesh Singh <brijesh.singh@amd.com> Reviewed-by: Laszlo Ersek <lersek@redhat.com> Regression-tested-by: Laszlo Ersek <lersek@redhat.com>
* Ovmfpkg/VirtioBlkDxe: map virtio-blk request and response buffersBrijesh Singh2017-08-281-13/+125
| | | | | | | | | | | | | | | | | | | | | | | | | | | | When device is behind the IOMMU, driver is require to pass the device address of virtio request, response and any memory referenced by those request/response to the bus master. The patch uses IOMMU-like member functions from VIRTIO_DEVICE_PROTOCOL to map request and response buffers system physical address to the device address. - If the buffer need to be accessed by both the processor and a bus master then map with BusMasterCommonBuffer. - If the buffer need to be accessed for a write operation by a bus master then map with BusMasterWrite. - If the buffer need to be accessed for a read operation by a bus master then map with BusMasterRead. Cc: Ard Biesheuvel <ard.biesheuvel@linaro.org> Cc: Jordan Justen <jordan.l.justen@intel.com> Cc: Tom Lendacky <thomas.lendacky@amd.com> Cc: Laszlo Ersek <lersek@redhat.com> Contributed-under: TianoCore Contribution Agreement 1.1 Signed-off-by: Brijesh Singh <brijesh.singh@amd.com> Reviewed-by: Laszlo Ersek <lersek@redhat.com> Regression-tested-by: Laszlo Ersek <lersek@redhat.com>
* OvmfPkg/VirtioBlkDxe: map VRING using VirtioRingMap()Brijesh Singh2017-08-282-8/+38
| | | | | | | | | | | | | | | When device is behind the IOMMU then driver need to pass the device address when programing the bus master. The patch uses VirtioRingMap() to map the VRING system physical address to device address. Cc: Ard Biesheuvel <ard.biesheuvel@linaro.org> Cc: Jordan Justen <jordan.l.justen@intel.com> Cc: Tom Lendacky <thomas.lendacky@amd.com> Cc: Laszlo Ersek <lersek@redhat.com> Contributed-under: TianoCore Contribution Agreement 1.1 Signed-off-by: Brijesh Singh <brijesh.singh@amd.com> Reviewed-by: Laszlo Ersek <lersek@redhat.com> Regression-tested-by: Laszlo Ersek <lersek@redhat.com>
* Revert "OvmfPkg/build.sh: select the GCC49 toolchain settings for gcc-7.*"Laszlo Ersek2017-08-251-1/+1
| | | | | | | | | | | | | | This reverts commit ca56256d5e0b7e63325b049e90a6bd03f90e3598: TianoCore BZ#671 <https://bugzilla.tianocore.org/show_bug.cgi?id=671> has been fixed in commit 2f7f1e73c10f ("BaseTools: Add the missing -pie link option in GCC tool chain", 2017-08-23), so we can return to the GCC5 toolchain with gcc-7.*. Cc: Jordan Justen <jordan.l.justen@intel.com> Contributed-under: TianoCore Contribution Agreement 1.1 Signed-off-by: Laszlo Ersek <lersek@redhat.com> Reviewed-by: Jordan Justen <jordan.l.justen@intel.com>
* OvmfPkg/VirtioRngDxe: negotiate VIRTIO_F_IOMMU_PLATFORMBrijesh Singh2017-08-251-2/+2
| | | | | | | | | | | | | | | | | VirtioRngDxe driver has been updated to use IOMMU-like member functions from VIRTIO_DEVICE_PROTOCOL to translate the system physical address to device address. We do not need to do anything special when VIRTIO_F_IOMMU_PLATFORM bit is present hence treat it in parallel with VIRTIO_F_VERSION_1. Cc: Ard Biesheuvel <ard.biesheuvel@linaro.org> Cc: Jordan Justen <jordan.l.justen@intel.com> Cc: Tom Lendacky <thomas.lendacky@amd.com> Cc: Laszlo Ersek <lersek@redhat.com> Contributed-under: TianoCore Contribution Agreement 1.1 Signed-off-by: Brijesh Singh <brijesh.singh@amd.com> Reviewed-by: Laszlo Ersek <lersek@redhat.com> Regression-tested-by: Laszlo Ersek <lersek@redhat.com>
* OvmfPkg/Virtio10: define VIRTIO_F_IOMMU_PLATFORM feature bitBrijesh Singh2017-08-251-1/+3
| | | | | | | | | | | | | | | | | | | This feature indicates that the device is behind an IOMMU that translates bus addresses from the device into physical addresses in memory. If this feature bit is set to 0, then the device emits physical addresses which are not translated further, even though an IOMMU may be present. see [1] for more infromation [1] https://lists.oasis-open.org/archives/virtio-dev/201610/msg00121.html Cc: Ard Biesheuvel <ard.biesheuvel@linaro.org> Cc: Jordan Justen <jordan.l.justen@intel.com> Cc: Tom Lendacky <thomas.lendacky@amd.com> Cc: Laszlo Ersek <lersek@redhat.com> Contributed-under: TianoCore Contribution Agreement 1.1 Signed-off-by: Brijesh Singh <brijesh.singh@amd.com> Reviewed-by: Laszlo Ersek <lersek@redhat.com> Regression-tested-by: Laszlo Ersek <lersek@redhat.com>
* OvmfPkg/VirtioRngDxe: map host address to device addressBrijesh Singh2017-08-252-9/+75
| | | | | | | | | | | | | | | | patch maps the host address to a device address for buffers (including rings, device specifc request and response pointed by vring descriptor, and any further memory reference by those request and response). Cc: Ard Biesheuvel <ard.biesheuvel@linaro.org> Cc: Jordan Justen <jordan.l.justen@intel.com> Cc: Tom Lendacky <thomas.lendacky@amd.com> Cc: Laszlo Ersek <lersek@redhat.com> Contributed-under: TianoCore Contribution Agreement 1.1 Signed-off-by: Brijesh Singh <brijesh.singh@amd.com> [lersek@redhat.com: return EFI_DEVICE_ERROR if mapping fails in GetRNG] Reviewed-by: Laszlo Ersek <lersek@redhat.com> Regression-tested-by: Laszlo Ersek <lersek@redhat.com>
* OvmfPkg/VirtioLib: change the parameter of VirtioAppendDesc() to UINT64Brijesh Singh2017-08-252-36/+37
| | | | | | | | | | | | | | | | | | | | | | The patch change the "BufferPhysAddr" parameter of VirtioAppendDesc() from type UINTN to UINT64. UINTN is appropriate as long as we pass system memory references. After the introduction of bus master device addresses, that's no longer the case in general. Should we implement "real" IOMMU support at some point, UINTN could break in 32-bit builds of OVMF. Suggested-by: Laszlo Ersek <lersek@redhat.com> Cc: Ard Biesheuvel <ard.biesheuvel@linaro.org> Cc: Jordan Justen <jordan.l.justen@intel.com> Cc: Tom Lendacky <thomas.lendacky@amd.com> Cc: Laszlo Ersek <lersek@redhat.com> Contributed-under: TianoCore Contribution Agreement 1.1 Signed-off-by: Brijesh Singh <brijesh.singh@amd.com> [lersek@redhat.com: clarify commit message] [lersek@redhat.com: balance parens in VirtioAppendDesc() comment blocks] Reviewed-by: Laszlo Ersek <lersek@redhat.com> Regression-tested-by: Laszlo Ersek <lersek@redhat.com>
* OvmfPkg/VirtioLib: alloc VRING buffer with AllocateSharedPages()Brijesh Singh2017-08-253-12/+16
| | | | | | | | | | | | | | | | The VRING buffer is a communication area between guest and hypervisor. Allocate it using VIRTIO_DEVICE_PROTOCOL.AllocateSharedPages() so that it can be mapped later with VirtioRingMap() for bi-directional access. Cc: Ard Biesheuvel <ard.biesheuvel@linaro.org> Cc: Jordan Justen <jordan.l.justen@intel.com> Cc: Tom Lendacky <thomas.lendacky@amd.com> Cc: Laszlo Ersek <lersek@redhat.com> Contributed-under: TianoCore Contribution Agreement 1.1 Signed-off-by: Brijesh Singh <brijesh.singh@amd.com> [lersek@redhat.com: correct typo in VirtioRingInit() comment blocks] Reviewed-by: Laszlo Ersek <lersek@redhat.com> Regression-tested-by: Laszlo Ersek <lersek@redhat.com>
* OvmfPkg/VirtioLib: add function to map VRINGBrijesh Singh2017-08-252-0/+71
| | | | | | | | | | | | | | | Add a function to map the ring buffer with BusMasterCommonBuffer so that ring can be accessed by both guest and hypervisor. Cc: Ard Biesheuvel <ard.biesheuvel@linaro.org> Cc: Jordan Justen <jordan.l.justen@intel.com> Cc: Tom Lendacky <thomas.lendacky@amd.com> Cc: Laszlo Ersek <lersek@redhat.com> Contributed-under: TianoCore Contribution Agreement 1.1 Signed-off-by: Brijesh Singh <brijesh.singh@amd.com> [lersek@redhat.com: fix typo in commit message] Reviewed-by: Laszlo Ersek <lersek@redhat.com> Regression-tested-by: Laszlo Ersek <lersek@redhat.com>
* OvmfPkg/Virtio10Dxe: add the RingBaseShift offsetBrijesh Singh2017-08-251-2/+3
| | | | | | | | | | | | | | | | virtio drivers use VIRTIO_DEVICE_PROTOCOL.MapSharedBuffer() to map the ring buffer host address to a device address. If an IOMMU is present then RingBaseShift contains the offset from the host address. Suggested-by: Laszlo Ersek <lersek@redhat.com> Cc: Ard Biesheuvel <ard.biesheuvel@linaro.org> Cc: Jordan Justen <jordan.l.justen@intel.com> Cc: Tom Lendacky <thomas.lendacky@amd.com> Cc: Laszlo Ersek <lersek@redhat.com> Contributed-under: TianoCore Contribution Agreement 1.1 Signed-off-by: Brijesh Singh <brijesh.singh@amd.com> Reviewed-by: Laszlo Ersek <lersek@redhat.com> Regression-tested-by: Laszlo Ersek <lersek@redhat.com>
* OvmfPkg/Virtio: take RingBaseShift in SetQueueAddress()Brijesh Singh2017-08-2511-12/+42
| | | | | | | | | | | | | | | | | | | | | | | | | For the case when an IOMMU is used for translating system physical addresses to DMA bus master addresses, the transport-independent virtio device drivers will be required to map their VRING areas to bus addresses with VIRTIO_DEVICE_PROTOCOL.MapSharedBuffer() calls. - MMIO and legacy virtio transport do not support IOMMU to translate the addresses hence RingBaseShift will always be set to zero. - modern virtio transport supports IOMMU to translate the address, in next patch we will update the Virtio10Dxe to use RingBaseShift offset. Suggested-by: Laszlo Ersek <lersek@redhat.com> Cc: Ard Biesheuvel <ard.biesheuvel@linaro.org> Cc: Jordan Justen <jordan.l.justen@intel.com> Cc: Tom Lendacky <thomas.lendacky@amd.com> Cc: Laszlo Ersek <lersek@redhat.com> Contributed-under: TianoCore Contribution Agreement 1.1 Signed-off-by: Brijesh Singh <brijesh.singh@amd.com> [lersek@redhat.com: remove commit msg paragraph with VirtioLib reference] [lersek@redhat.com: fix typo in VIRTIO_SET_QUEUE_ADDRESS comment block] Reviewed-by: Laszlo Ersek <lersek@redhat.com> Regression-tested-by: Laszlo Ersek <lersek@redhat.com>
* OvmfPkg/VirtioLib: take VirtIo instance in VirtioRingInit/VirtioRingUninitBrijesh Singh2017-08-258-26/+44
| | | | | | | | | | | | | | Passing the VirtIo protocol instance will allow the vring to use VIRTIO_DEVICE_PROTOCOL.AllocateSharedPages () to allocate vring buffer. Cc: Ard Biesheuvel <ard.biesheuvel@linaro.org> Cc: Jordan Justen <jordan.l.justen@intel.com> Cc: Tom Lendacky <thomas.lendacky@amd.com> Cc: Laszlo Ersek <lersek@redhat.com> Contributed-under: TianoCore Contribution Agreement 1.1 Signed-off-by: Brijesh Singh <brijesh.singh@amd.com> Reviewed-by: Laszlo Ersek <lersek@redhat.com> Regression-tested-by: Laszlo Ersek <lersek@redhat.com>
* OvmfPkg/VirtioLib: add VirtioMapAllBytesInSharedBuffer() helper functionBrijesh Singh2017-08-252-0/+137
| | | | | | | | | | | | | | | | | The function can be used for mapping the system physical address to virtio device address using VIRTIO_DEVICE_PROTOCOL.MapSharedBuffer (). The function helps with centralizing error handling, and it allows the caller to pass in constant or other evaluated expressions for NumberOfBytes. Cc: Ard Biesheuvel <ard.biesheuvel@linaro.org> Cc: Jordan Justen <jordan.l.justen@intel.com> Cc: Tom Lendacky <thomas.lendacky@amd.com> Cc: Laszlo Ersek <lersek@redhat.com> Contributed-under: TianoCore Contribution Agreement 1.1 Signed-off-by: Brijesh Singh <brijesh.singh@amd.com> [lersek@redhat.com: s/This/VirtIo/ in the new function's comment blocks] Reviewed-by: Laszlo Ersek <lersek@redhat.com> Regression-tested-by: Laszlo Ersek <lersek@redhat.com>
* OvmfPkg/VirtioMmioDeviceLib: implement IOMMU-like member functionsBrijesh Singh2017-08-253-2/+99
| | | | | | | | | | | | | | | | | | | | | | | | | The patch implements the newly added IOMMU-like member functions by respectively delegating the job to: - VIRTIO_DEVICE_PROTOCOL.AllocateSharedPages () -> MemoryAllocationLib.AllocatePages() - VIRTIO_DEVICE_PROTOCOL.FreeSharedPages () -> MemoryAllocationLib.FreePages () - VIRTIO_DEVICE_PROTOCOL.MapSharedBuffer () -> no-op - VIRTIO_DEVICE_PROTOCOL.UnmapSharedBuffer () -> no-op Suggested-by: Laszlo Ersek <lersek@redhat.com> Cc: Ard Biesheuvel <ard.biesheuvel@linaro.org> Cc: Jordan Justen <jordan.l.justen@intel.com> Cc: Tom Lendacky <thomas.lendacky@amd.com> Cc: Laszlo Ersek <lersek@redhat.com> Contributed-under: TianoCore Contribution Agreement 1.1 Signed-off-by: Brijesh Singh <brijesh.singh@amd.com> Reviewed-by: Laszlo Ersek <lersek@redhat.com> Regression-tested-by: Laszlo Ersek <lersek@redhat.com>
* OvmfPkg/VirtioPciDeviceDxe: implement IOMMU-like member functionsBrijesh Singh2017-08-253-1/+98
| | | | | | | | | | | | | | | | | | | | | | | | | The patch implements the newly added IOMMU-like member functions by respectively delegating the job to: - VIRTIO_DEVICE_PROTOCOL.AllocateSharedPages () -> MemoryAllocationLib.AllocatePages() - VIRTIO_DEVICE_PROTOCOL.FreeSharedPages () -> MemoryAllocationLib.FreePages () - VIRTIO_DEVICE_PROTOCOL.MapSharedBuffer () -> no-op - VIRTIO_DEVICE_PROTOCOL.UnmapSharedBuffer () -> no-op Suggested-by: Laszlo Ersek <lersek@redhat.com> Cc: Ard Biesheuvel <ard.biesheuvel@linaro.org> Cc: Jordan Justen <jordan.l.justen@intel.com> Cc: Tom Lendacky <thomas.lendacky@amd.com> Cc: Laszlo Ersek <lersek@redhat.com> Contributed-under: TianoCore Contribution Agreement 1.1 Signed-off-by: Brijesh Singh <brijesh.singh@amd.com> Reviewed-by: Laszlo Ersek <lersek@redhat.com> Regression-tested-by: Laszlo Ersek <lersek@redhat.com>
* OvmfPkg/Virtio10Dxe: implement IOMMU-like member functionsBrijesh Singh2017-08-251-2/+120
| | | | | | | | | | | | | | | | | | | | | | | | | | | The patch implements the newly added IOMMU-like member functions by respectively delegating the job to: - VIRTIO_DEVICE_PROTOCOL.AllocateSharedPages() -> EFI_PCI_IO_PROTOCOL.AllocateBuffer() - VIRTIO_DEVICE_PROTOCOL.FreeSharedPages() -> EFI_PCI_IO_PROTOCOL.FreeBuffer() - VIRTIO_DEVICE_PROTOCOL.MapSharedBuffer() -> EFI_PCI_IO_PROTOCOL.Map() - VIRTIO_DEVICE_PROTOCOL.UnmapSharedBuffer() -> EFI_PCI_IO_PROTOCOL.Unmap() Suggested-by: Laszlo Ersek <lersek@redhat.com> Cc: Ard Biesheuvel <ard.biesheuvel@linaro.org> Cc: Jordan Justen <jordan.l.justen@intel.com> Cc: Tom Lendacky <thomas.lendacky@amd.com> Cc: Laszlo Ersek <lersek@redhat.com> Contributed-under: TianoCore Contribution Agreement 1.1 Signed-off-by: Brijesh Singh <brijesh.singh@amd.com> Reviewed-by: Laszlo Ersek <lersek@redhat.com> Regression-tested-by: Laszlo Ersek <lersek@redhat.com>
* OvmfPkg: introduce IOMMU-like member functions to VIRTIO_DEVICE_PROTOCOLBrijesh Singh2017-08-251-0/+143
| | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | The patch extends VIRTIO_DEVICE_PROTOCOL to provide the following new member functions: - AllocateSharedPages : allocate a memory region suitable for sharing between guest and hypervisor (e.g ring buffer). - FreeSharedPages: free the memory allocated using AllocateSharedPages (). - MapSharedBuffer: map a host address to device address suitable to share with device for bus master operations. - UnmapSharedBuffer: unmap the device address obtained through the MapSharedBuffer(). We're free to extend the protocol structure without changing the protocol GUID, or bumping any protocol version fields (of which we currently have none), because VIRTIO_DEVICE_PROTOCOL is internal to edk2 by design -- see the disclaimers in "VirtioDevice.h". The patch implements Laszlo's recommendation [1]. [1] http://mid.mail-archive.com/841bec5f-6f6e-8b1f-25ba-0fd37a915b72@redhat.com Suggested-by: Laszlo Ersek <lersek@redhat.com> Cc: Ard Biesheuvel <ard.biesheuvel@linaro.org> Cc: Jordan Justen <jordan.l.justen@intel.com> Cc: Tom Lendacky <thomas.lendacky@amd.com> Cc: Laszlo Ersek <lersek@redhat.com> Contributed-under: TianoCore Contribution Agreement 1.1 Signed-off-by: Brijesh Singh <brijesh.singh@amd.com> Reviewed-by: Laszlo Ersek <lersek@redhat.com> Regression-tested-by: Laszlo Ersek <lersek@redhat.com>
* OvmfPkg/QemuVideoDxe: remove AARCH64/ARM supportArd Biesheuvel2017-08-241-2/+2
| | | | | | | | | | | | Now that we have dropped QemuVideoDxe from all QEMU targeted builds under ArmVirtPkg, we can revert the ARM specific changes to it. This partially reverts commits 84a75f70e903 (SVN 16890) and 05a537945872. Contributed-under: TianoCore Contribution Agreement 1.1 Signed-off-by: Ard Biesheuvel <ard.biesheuvel@linaro.org> Reviewed-by: Laszlo Ersek <lersek@redhat.com>
* OvmfPkg/build.sh: select the GCC49 toolchain settings for gcc-7.*Laszlo Ersek2017-08-171-1/+1
| | | | | | | | | | | | | | | | | | | When UefiCpuPkg/MpInitLib is built for X64 with gcc-7, using the DEBUG build target and the GCC5 toolchain settings, a C-language assignment is miscompiled such that the initial AP startup hangs in CpuMpPei (X64) or CpuDxe (Ia32X64). See <https://bugzilla.tianocore.org/show_bug.cgi?id=671> for a detailed analysis of the symptoms, and for mailing list links. This issue has been reported several times (one example is <https://bugzilla.tianocore.org/show_bug.cgi?id=657>). Until we (or the upstream gcc developers) figure out how to dissuade gcc-7 from the miscompilation, pick the GCC49 toolchain in "build.sh" for gcc-7.*. Cc: Alex Williamson <alex.williamson@redhat.com> Cc: Jordan Justen <jordan.l.justen@intel.com> Contributed-under: TianoCore Contribution Agreement 1.1 Signed-off-by: Laszlo Ersek <lersek@redhat.com> Reviewed-by: Jordan Justen <jordan.l.justen@intel.com>
* OvmfPkg/Protocol/VirtioDevice: fix comment styleBrijesh Singh2017-08-151-3/+11
| | | | | | | | | | | | | Cc: Ard Biesheuvel <ard.biesheuvel@linaro.org> Cc: Jordan Justen <jordan.l.justen@intel.com> Cc: Tom Lendacky <thomas.lendacky@amd.com> Cc: Laszlo Ersek <lersek@redhat.com> Contributed-under: TianoCore Contribution Agreement 1.1 Signed-off-by: Brijesh Singh <brijesh.singh@amd.com> [lersek@redhat.com: clarify subject line] [lersek@redhat.com: adjust the set of comments updated by the patch] Reviewed-by: Laszlo Ersek <lersek@redhat.com> Regression-tested-by: Laszlo Ersek <lersek@redhat.com>
* OvmfPkg/VirtioMmioDeviceLib: add missing IN and OUT decorationBrijesh Singh2017-08-152-28/+28
| | | | | | | | | | | Cc: Ard Biesheuvel <ard.biesheuvel@linaro.org> Cc: Jordan Justen <jordan.l.justen@intel.com> Cc: Tom Lendacky <thomas.lendacky@amd.com> Cc: Laszlo Ersek <lersek@redhat.com> Contributed-under: TianoCore Contribution Agreement 1.1 Signed-off-by: Brijesh Singh <brijesh.singh@amd.com> Reviewed-by: Laszlo Ersek <lersek@redhat.com> Regression-tested-by: Laszlo Ersek <lersek@redhat.com>
* OvmfPkg/VirtioPciDeviceDxe: add missing IN and OUT decorationBrijesh Singh2017-08-152-24/+24
| | | | | | | | | | | Cc: Ard Biesheuvel <ard.biesheuvel@linaro.org> Cc: Jordan Justen <jordan.l.justen@intel.com> Cc: Tom Lendacky <thomas.lendacky@amd.com> Cc: Laszlo Ersek <lersek@redhat.com> Contributed-under: TianoCore Contribution Agreement 1.1 Signed-off-by: Brijesh Singh <brijesh.singh@amd.com> Reviewed-by: Laszlo Ersek <lersek@redhat.com> Regression-tested-by: Laszlo Ersek <lersek@redhat.com>
* OvmfPkg/Virtio10Dxe: supply missing BUS_MASTER attributeBrijesh Singh2017-08-151-1/+1
| | | | | | | | | | | | | | | | | Virtio devices read and write guest RAM (they don't just decode their IO and/or MMIO BARs), which translates to "bus master". Suggested-by: Laszlo Ersek <lersek@redhat.com> Cc: Ard Biesheuvel <ard.biesheuvel@linaro.org> Cc: Jordan Justen <jordan.l.justen@intel.com> Cc: Tom Lendacky <thomas.lendacky@amd.com> Cc: Laszlo Ersek <lersek@redhat.com> Contributed-under: TianoCore Contribution Agreement 1.1 Signed-off-by: Brijesh Singh <brijesh.singh@amd.com> [lersek@redhat.com: expand commit message body] [lersek@redhat.com: remove superfluous whitespace in assignment] Reviewed-by: Laszlo Ersek <lersek@redhat.com> Regression-tested-by: Laszlo Ersek <lersek@redhat.com>
* OvmfPkg/VirtioPciDeviceDxe: supply missing BUS_MASTER attributeBrijesh Singh2017-08-151-3/+7
| | | | | | | | | | | | | | | | | Virtio devices read and write guest RAM (they don't just decode their IO and/or MMIO BARs), which translates to "bus master". Suggested-by: Laszlo Ersek <lersek@redhat.com> Cc: Ard Biesheuvel <ard.biesheuvel@linaro.org> Cc: Jordan Justen <jordan.l.justen@intel.com> Cc: Tom Lendacky <thomas.lendacky@amd.com> Cc: Laszlo Ersek <lersek@redhat.com> Contributed-under: TianoCore Contribution Agreement 1.1 Signed-off-by: Brijesh Singh <brijesh.singh@amd.com> [lersek@redhat.com: expand commit message body] [lersek@redhat.com: fix up line breaking style (original code too)] Reviewed-by: Laszlo Ersek <lersek@redhat.com> Regression-tested-by: Laszlo Ersek <lersek@redhat.com>
* OvmfPkg/AcpiPlatformDxe: short-circuit the transfer of an empty S3_CONTEXTLaszlo Ersek2017-08-081-1/+9
| | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | In commit 805762252733 ("OvmfPkg/AcpiPlatformDxe: save fw_cfg boot script with QemuFwCfgS3Lib", 2017-02-23), we replaced the explicit S3 boot script manipulation in TransferS3ContextToBootScript() with a call to QemuFwCfgS3CallWhenBootScriptReady(). (Passing AppendFwCfgBootScript() as callback.) QemuFwCfgS3CallWhenBootScriptReady() checks for fw_cfg DMA up-front, and bails with RETURN_NOT_FOUND if fw_cfg DMA is missing. (This is justified as the goal of QemuFwCfgS3Lib is to "enable[] driver modules [...] to produce fw_cfg DMA operations that are to be replayed at S3 resume time".) In turn, if QemuFwCfgS3CallWhenBootScriptReady() fails, then OvmfPkg/AcpiPlatformDxe rolls back any earlier linker/loader script processing, and falls back to the built-in ACPI tables. (This is also justified because failure to save WRITE_POINTER commands for replaying at S3 resume implies failure to process the linker/loader script comprehensively.) Calling QemuFwCfgS3CallWhenBootScriptReady() from TransferS3ContextToBootScript() *unconditionally* is wrong however. For the case when the linker/loader script contains no WRITE_POINTER commands, the call perpetuated an earlier side effect, and introduced another one: (1) On machine types that provide fw_cfg DMA (i.e., 2.5+), QemuFwCfgS3CallWhenBootScriptReady() would succeed, and allocate workspace for the boot script opcodes in reserved memory. However, no opcodes would actually be produced in the AppendFwCfgBootScript() callback, due to lack of any WRITE_POINTER commands. This waste of reserved memory had been introduced in earlier commit df73df138d9d ("OvmfPkg/AcpiPlatformDxe: replay QEMU_LOADER_WRITE_POINTER commands at S3", 2017-02-09). (2) On machine types that lack fw_cfg DMA (i.e., 2.4 and earlier), TransferS3ContextToBootScript() would now fail the linker/loader script for no reason. (Note that QEMU itself prevents adding devices that depend on WRITE_POINTER if the machine type lacks fw_cfg DMA: $ qemu-system-x86_64 -M pc-q35-2.4 -device vmgenid qemu-system-x86_64: -device vmgenid: vmgenid requires DMA write support in fw_cfg, which this machine type does not provide) Short-circuit an empty S3_CONTEXT in TransferS3ContextToBootScript() by dropping S3_CONTEXT on the floor. This is compatible with the current contract of the function as it constitutes a transfer of ownership. Regression (2) was found and reported by Dhiru Kholia as an OSX guest boot failure on the "pc-q35-2.4" machine type: http://mid.mail-archive.com/CANO7a6x6EaWNZ8y=MvLU=w_LjRLXserO3NmsgHvaYE0aUCCWzg@mail.gmail.com Dhiru bisected the issue to commit 805762252733. Cc: Dhiru Kholia <dhiru.kholia@gmail.com> Cc: Jordan Justen <jordan.l.justen@intel.com> Fixes: df73df138d9d53f7f7570f4fe97a6cde941a2656 Fixes: 805762252733bb67bc5157f0137c64e010724c77 Reported-by: Dhiru Kholia <dhiru.kholia@gmail.com> Contributed-under: TianoCore Contribution Agreement 1.1 Signed-off-by: Laszlo Ersek <lersek@redhat.com> Tested-by: Dhiru Kholia <dhiru.kholia@gmail.com> Reviewed-by: Jordan Justen <jordan.l.justen@intel.com>
* OvmfPkg/PlatformPei: support >=1TB high RAM, and discontiguous high RAMLaszlo Ersek2017-08-051-2/+127
| | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | In OVMF we currently get the upper (>=4GB) memory size with the GetSystemMemorySizeAbove4gb() function. The GetSystemMemorySizeAbove4gb() function is used in two places: (1) It is the starting point of the calculations in GetFirstNonAddress(). GetFirstNonAddress() in turn - determines the placement of the 64-bit PCI MMIO aperture, - provides input for the GCD memory space map's sizing (see AddressWidthInitialization(), and the CPU HOB in MiscInitialization()), - influences the permanent PEI RAM cap (the DXE core's page tables, built in permanent PEI RAM, grow as the RAM to map grows). (2) In QemuInitializeRam(), GetSystemMemorySizeAbove4gb() determines the single memory descriptor HOB that we produce for the upper memory. Respectively, there are two problems with GetSystemMemorySizeAbove4gb(): (1) It reads a 24-bit count of 64KB RAM chunks from the CMOS, and therefore cannot return a larger value than one terabyte. (2) It cannot express discontiguous high RAM. Starting with version 1.7.0, QEMU has provided the fw_cfg file called "etc/e820". Refer to the following QEMU commits: - 0624c7f916b4 ("e820: pass high memory too.", 2013-10-10), - 7d67110f2d9a ("pc: add etc/e820 fw_cfg file", 2013-10-18) - 7db16f2480db ("pc: register e820 entries for ram", 2013-10-10) Ever since these commits in v1.7.0 -- with the last QEMU release being v2.9.0, and v2.10.0 under development --, the only two RAM entries added to this E820 map correspond to the below-4GB RAM range, and the above-4GB RAM range. And, the above-4GB range exactly matches the CMOS registers in question; see the use of "pcms->above_4g_mem_size": pc_q35_init() | pc_init1() pc_memory_init() e820_add_entry(0x100000000ULL, pcms->above_4g_mem_size, E820_RAM); pc_cmos_init() val = pcms->above_4g_mem_size / 65536; rtc_set_memory(s, 0x5b, val); rtc_set_memory(s, 0x5c, val >> 8); rtc_set_memory(s, 0x5d, val >> 16); Therefore, remedy the above OVMF limitations as follows: (1) Start off GetFirstNonAddress() by scanning the E820 map for the highest exclusive >=4GB RAM address. Fall back to the CMOS if the E820 map is unavailable. Base all further calculations (such as 64-bit PCI MMIO aperture placement, GCD sizing etc) on this value. At the moment, the only difference this change makes is that we can have more than 1TB above 4GB -- given that the sole "high RAM" entry in the E820 map matches the CMOS exactly, modulo the most significant bits (see above). However, Igor plans to add discontiguous (cold-plugged) high RAM to the fw_cfg E820 RAM map later on, and then this scanning will adapt automatically. (2) In QemuInitializeRam(), describe the high RAM regions from the E820 map one by one with memory HOBs. Fall back to the CMOS only if the E820 map is missing. Again, right now this change only makes a difference if there is at least 1TB high RAM. Later on it will adapt to discontiguous high RAM (regardless of its size) automatically. -*- Implementation details: introduce the ScanOrAdd64BitE820Ram() function, which reads the E820 entries from fw_cfg, and finds the highest exclusive >=4GB RAM address, or produces memory resource descriptor HOBs for RAM entries that start at or above 4GB. The RAM map is not read in a single go, because its size can vary, and in PlatformPei we should stay away from dynamic memory allocation, for the following reasons: - "Pool" allocations are limited to ~64KB, are served from HOBs, and cannot be released ever. - "Page" allocations are seriously limited before PlatformPei installs the permanent PEI RAM. Furthermore, page allocations can only be released in DXE, with dedicated code (so the address would have to be passed on with a HOB or PCD). - Raw memory allocation HOBs would require the same freeing in DXE. Therefore we process each E820 entry as soon as it is read from fw_cfg. -*- Considering the impact of high RAM on the DXE core: A few years ago, installing high RAM as *tested* would cause the DXE core to inhabit such ranges rather than carving out its home from the permanent PEI RAM. Fortunately, this was fixed in the following edk2 commit: 3a05b13106d1, "MdeModulePkg DxeCore: Take the range in resource HOB for PHIT as higher priority", 2015-09-18 which I regression-tested at the time: http://mid.mail-archive.com/55FC27B0.4070807@redhat.com Later on, OVMF was changed to install its high RAM as tested (effectively "arming" the earlier DXE core change for OVMF), in the following edk2 commit: 035ce3b37c90, "OvmfPkg/PlatformPei: Add memory above 4GB as tested", 2016-04-21 which I also regression-tested at the time: http://mid.mail-archive.com/571E8B90.1020102@redhat.com Therefore adding more "tested memory" HOBs is safe. Cc: Jordan Justen <jordan.l.justen@intel.com> Ref: https://bugzilla.redhat.com/show_bug.cgi?id=1468526 Contributed-under: TianoCore Contribution Agreement 1.1 Signed-off-by: Laszlo Ersek <lersek@redhat.com> Reviewed-by: Jordan Justen <jordan.l.justen@intel.com>
* OvmfPkg/QemuFwCfgLib: Use BusMasterCommonBuffer to map FW_CFG_DMA_ACCESSBrijesh Singh2017-08-055-292/+367
| | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | Commit 09719a01b11b (OvmfPkg/QemuFwCfgLib: Implement SEV internal function for Dxe phase) uses IOMMU protocol to allocate and free FW_CFG_DMA_ACCESS buffer when SEV is active. During initial commits we made assumption that IOMMU.AllocateBuffer() will provide PlainTextAddress (i.e C-bit cleared). This assumption was wrong, the AllocateBuffer() protocol member is not expected to produce a buffer that is immediatly usable, and client is required to call Map() uncondtionally with BusMasterCommonBuffer[64] to get a mapping which is accessable by both host and device. The patch refactors code a bit and add the support to Map() FW_CFG_DMA_ACCESS buffer using BusMasterCommonBuffer operation after allocation and Unamp() before free. The complete discussion about this and recommendation from Laszlo can be found here [1] [1] https://lists.01.org/pipermail/edk2-devel/2017-July/012652.html Suggested-by: Laszlo Ersek <lersek@redhat.com> Cc: Jordan Justen <jordan.l.justen@intel.com> Cc: Laszlo Ersek <lersek@redhat.com> Contributed-under: TianoCore Contribution Agreement 1.0 Signed-off-by: Brijesh Singh <brijesh.singh@amd.com> [lersek@redhat.com: convert pointers to UINTN before converting to UINT64] [lersek@redhat.com: fix argument indentation in multi-line function call] [lersek@redhat.com: explicitly compare pointers to NULL] Reviewed-by: Laszlo Ersek <lersek@redhat.com> Regression-tested-by: Laszlo Ersek <lersek@redhat.com> Contributed-under: TianoCore Contribution Agreement 1.1 Signed-off-by: Laszlo Ersek <lersek@redhat.com>
* OvmfPkg/IoMmuDxe: Unmap(): recycle MAP_INFO after BusMasterCommonBuffer[64]Laszlo Ersek2017-08-051-8/+40
| | | | | | | | | | | | | | | | | | | | | In order for Unmap() to be callable from ExitBootServices() event handler context (for cleaning up a BusMasterCommonBuffer[64] operation), we have to completely liberate the affected path in Unmap() from dynamic memory management. The last remaining piece is the release of the MAP_INFO structure. Rather than freeing it with FreePool(), recycle it to an internal list. Elements of this "free list" can be reused for any kind of Map() operation, and can be freed later, or recycled again. 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.0 Signed-off-by: Laszlo Ersek <lersek@redhat.com> Tested-by: Brijesh Singh <brijesh.singh@amd.com> Reviewed-by: Brijesh Singh <brijesh.singh@amd.com>
* OvmfPkg/IoMmuDxe: abort harder on memory encryption mask failuresLaszlo Ersek2017-08-051-2/+8
| | | | | | | | | | | | | | | | | | | | | | Upon a MemEncryptSevClearPageEncMask() failure in Map(), it wouldn't be difficult to release the bounce buffer that was implicitly allocated for BusMasterRead[64] and BusMasterWrite[64] operations. However, undoing any partial memory encryption mask changes -- partial page splitting and PTE modifications -- is practically impossible. (For example, restoring the encryption mask on the entire range has no reason to fare any better than the MemEncryptSevClearPageEncMask() call itself.) For this reason, keep ASSERT_EFI_ERROR(), but hang in RELEASE builds too, if MemEncryptSevClearPageEncMask() or MemEncryptSevSetPageEncMask() fails. 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.0 Signed-off-by: Laszlo Ersek <lersek@redhat.com> Tested-by: Brijesh Singh <brijesh.singh@amd.com> Reviewed-by: Brijesh Singh <brijesh.singh@amd.com>
* OvmfPkg/IoMmuDxe: implement in-place decryption/encryption for Map/UnmapLaszlo Ersek2017-08-051-56/+189
| | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | At the moment, we have the following distribution of actions between the IOMMU protocol member functions: - AllocateBuffer() allocates pages and clears the memory encryption mask. - FreeBuffer() re-sets the memory encryption mask, and deallocates pages. - Map() does nothing at all when BusMasterCommonBuffer[64] is requested (and AllocateBuffer() was called previously). Otherwise, Map() allocates pages, and clears the memory encryption mask. - Unmap() does nothing when cleaning up a BusMasterCommonBuffer[64] operation. Otherwise, Unmap() clears the encryption mask, and frees the pages. This is wrong: the AllocateBuffer() protocol member is not expected to produce a buffer that is immediately usable, and client code is required to call Map() unconditionally, even if BusMasterCommonBuffer[64] is the desired operation. Implement the right distribution of actions as follows: - AllocateBuffer() allocates pages and does not touch the encryption mask. - FreeBuffer() deallocates pages and does not touch the encryption mask. - Map() does not allocate pages when BusMasterCommonBuffer[64] is requested, and it allocates pages (bounce buffer) otherwise. Regardless of the BusMaster operation, Map() (and Map() only) clears the memory encryption mask. - Unmap() restores the encryption mask unconditionally. If the operation was BusMasterCommonBuffer[64], then Unmap() does not release the pages. Otherwise, the pages (bounce buffer) are released. This approach also ensures that Unmap() can be called from ExitBootServices() event handlers, for cleaning up BusMasterCommonBuffer[64] operations. (More specifically, for restoring the SEV encryption mask on any in-flight buffers, after resetting any referring devices.) ExitBootServices() event handlers must not change the UEFI memory map, thus any memory allocation or freeing in Unmap() would disqualify Unmap() from being called in such a context. Map()-ing and Unmap()-ing memory for a BusMasterCommonBuffer[64] operation effectively means in-place decryption and encryption in a SEV context. As an additional hurdle, section "7.10.8 Encrypt-in-Place" of AMD publication Nr.24593 implies that we need a separate temporary buffer for decryption and encryption that will eventually land in-place. Allocating said temporary buffer in the straightforward way would violate the above allocation/freeing restrictions on Map()/Unmap(), therefore pre-allocate this "stash buffer" too in AllocateBuffer(), and free it in FreeBuffer(). To completely rid Unmap() of dynamic memory impact, for BusMasterCommonBuffer[64] operations, we're going to rework the lifecycle of the MAP_INFO structures in a later patch. (The MemEncryptSevSetPageEncMask() call in Unmap() could theoretically allocate memory internally for page splitting, however this won't happen in practice: in Unmap() we only restore the memory encryption mask, and don't genuinely set it. Any page splitting will have occurred in Map()'s MemEncryptSevClearPageEncMask() call first.) 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.0 Signed-off-by: Laszlo Ersek <lersek@redhat.com> Tested-by: Brijesh Singh <brijesh.singh@amd.com> Reviewed-by: Brijesh Singh <brijesh.singh@amd.com>
* OvmfPkg/IoMmuDxe: rework setup of "MapInfo->PlainTextAddress" in Map()Laszlo Ersek2017-08-051-69/+87
| | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | There are three issues with the current calculations: - The initial logic that sets up "DmaMemoryTop" and "AllocateType" checks for the BusMasterCommonBuffer64 operation in two places. The inner check for BusMasterCommonBuffer64 will never evaluate to TRUE however, because the outer check excludes BusMasterCommonBuffer64. - In order to lower "DmaMemoryTop" to (SIZE_4GB - 1), the outer check requires that the encrypted (original) buffer cross the 4GB mark. This is wrong: for BusMasterRead[64] and BusMasterWrite[64] operations, we unconditionally need a bounce buffer (a decrypted memory area), and for the 32-bit variants, "DmaMemoryTop" should be lowered regardless of the location of the original (encrypted) buffer. - The current logic would be hard to extend for the in-place decryption that we'll implement in the next patch. Therefore rework the "MapInfo->PlainTextAddress" setup. No functional changes beyond said bugfixes. 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.0 Signed-off-by: Laszlo Ersek <lersek@redhat.com> Tested-by: Brijesh Singh <brijesh.singh@amd.com> Reviewed-by: Brijesh Singh <brijesh.singh@amd.com>
* OvmfPkg/IoMmuDxe: zero out pages before releasing themLaszlo Ersek2017-08-051-0/+5
| | | | | | | | | | | | | | | | | | | | | | | Whenever we release the plaintext bounce buffer pages that were allocated implicitly in Map() for BusMasterRead[64] and BusMasterWrite[64], we restore the encryption mask on them. However, we should also rewrite the area (fill it with zeros) so that the hypervisor is not left with a plaintext view of the earlier data. Similarly, whenever we release the plaintext common buffer pages that were allocated explicitly in AllocateBuffer() for BusMasterCommonBuffer[64], we restore the encryption mask on them. However, we should also rewrite the area (fill it with zeros) so that the hypervisor is not left with a plaintext view of the earlier data. 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.0 Signed-off-by: Laszlo Ersek <lersek@redhat.com> Tested-by: Brijesh Singh <brijesh.singh@amd.com> Reviewed-by: Brijesh Singh <brijesh.singh@amd.com>
* OvmfPkg/IoMmuDxe: clean up used library classesLaszlo Ersek2017-08-053-15/+6
| | | | | | | | | | | | | | | | | | | | | | | | | | | | The following library classes are not used by this module, so remove them from the INF file's [LibraryClasses] section: - DxeServicesTableLib - UefiLib The following library classes are used by this module, so add them to the INF file's [LibraryClasses] section: - BaseMemoryLib (e.g. via CopyMem()) - MemoryAllocationLib (e.g. via AllocatePool()) Sort the list of library classes (in both "IoMmuDxe.inf" and "AmdSevIoMmu.h"). Remove all non-local #include directives from "IoMmuDxe.c"; both C files of this module include "AmdSevIoMmu.h", and "AmdSevIoMmu.h" includes all non-local headers already. 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.0 Signed-off-by: Laszlo Ersek <lersek@redhat.com> Tested-by: Brijesh Singh <brijesh.singh@amd.com> Reviewed-by: Brijesh Singh <brijesh.singh@amd.com>
* OvmfPkg/IoMmuDxe: propagate errors from AmdSevInstallIoMmuProtocol()Laszlo Ersek2017-08-053-6/+4
| | | | | | | | | | | | | | | If we cannot install the IOMMU protocol for whatever reason, exit the driver with an error. The same is already done for the IOMMU Absent protocol. 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.0 Signed-off-by: Laszlo Ersek <lersek@redhat.com> Tested-by: Brijesh Singh <brijesh.singh@amd.com> Reviewed-by: Brijesh Singh <brijesh.singh@amd.com>
* OvmfPkg/IoMmuDxe: don't initialize local variablesLaszlo Ersek2017-08-051-2/+6
| | | | | | | | | | | | | The edk2 coding style requires separate assignments. 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.0 Signed-off-by: Laszlo Ersek <lersek@redhat.com> Tested-by: Brijesh Singh <brijesh.singh@amd.com> Reviewed-by: Brijesh Singh <brijesh.singh@amd.com>
* OvmfPkg/IoMmuDxe: convert UINTN arguments to UINT64 for the %Lx fmt specLaszlo Ersek2017-08-051-7/+7
| | | | | | | | | | | | | | | The portable way to print UINTN values is to use the %Lx format specifier, and to convert the values to UINT64. The second step is currently missing, add it. 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.0 Signed-off-by: Laszlo Ersek <lersek@redhat.com> Tested-by: Brijesh Singh <brijesh.singh@amd.com> Reviewed-by: Brijesh Singh <brijesh.singh@amd.com>
* OvmfPkg/IoMmuDxe: rename HostAddress to CryptedAddress in MAP_INFOLaszlo Ersek2017-08-051-8/+8
| | | | | | | | | | | | | | As a continuation of the last patch, clarify that the area pointed-to by "HostAddress" is encrypted and hidden from the hypervisor. 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.0 Signed-off-by: Laszlo Ersek <lersek@redhat.com> Tested-by: Brijesh Singh <brijesh.singh@amd.com> Reviewed-by: Brijesh Singh <brijesh.singh@amd.com>
* OvmfPkg/IoMmuDxe: rename DeviceAddress to PlainTextAddress in MAP_INFOLaszlo Ersek2017-08-051-13/+13
| | | | | | | | | | | | | | | | | In this particular IOMMU driver, "DeviceAddress" is just as accessible to the CPU as "HostAddress", the difference is that the area pointed-to by the former is plain-text and accessible to the hypervisor. Rename "DeviceAddress" to "PlainTextAddress" in MAP_INFO. 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> Suggested-by: Ard Biesheuvel <ard.biesheuvel@linaro.org> Contributed-under: TianoCore Contribution Agreement 1.0 Signed-off-by: Laszlo Ersek <lersek@redhat.com> Tested-by: Brijesh Singh <brijesh.singh@amd.com> Reviewed-by: Brijesh Singh <brijesh.singh@amd.com>
* OvmfPkg/IoMmuDxe: rewrap source code to 79 charactersLaszlo Ersek2017-08-054-70/+130
| | | | | | | | | | | | | No functional changes. 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.0 Signed-off-by: Laszlo Ersek <lersek@redhat.com> Tested-by: Brijesh Singh <brijesh.singh@amd.com> Reviewed-by: Brijesh Singh <brijesh.singh@amd.com>
* OvmfPkg/IoMmuDxe: Fix header guard macroThomas Palmer2017-08-051-2/+3
| | | | | | | | Correct the header guard macro Contributed-under: TianoCore Contribution Agreement 1.0 Signed-off-by: Thomas Palmer <thomas.palmer@hpe.com> Reviewed-by: Laszlo Ersek <lersek@redhat.com>