On 27 July 2017 at 23:10, Brijesh Singh <[email protected]> wrote: > > > On 07/27/2017 04:31 PM, Ard Biesheuvel wrote: >> >> >>> On 27 Jul 2017, at 21:55, Brijesh Singh <[email protected]> wrote: >>> >>> >>> >>> On 07/27/2017 02:00 PM, Brijesh Singh wrote: >>> >>>>>> This distribution of operations seems wrong. The key point is that >>>>>> AllocateBuffer() *need not* result in a buffer that is immediately >>>>>> usable, and that client code is required to call Map() >>>>>> *unconditionally*, even if BusMasterCommonBuffer is the desired >>>>>> operation. Therefore, the right distribution of operations is: >>>>>> >>>>>> - IoMmuAllocateBuffer() allocates pages and does not touch the >>>>>> encryption mask.. >>>>>> >>>>>> - IoMmuFreeBuffer() deallocates pages and does not touch the >>>>>> encryption >>>>>> mask. >>>>>> >>>> Actually one of main reason why we cleared and restored the memory >>>> encryption mask >>>> during allocate/free is because we also consume the IOMMU protocol in >>>> QemuFwCfgLib >>>> as a method to allocate and free a DMA buffer. I am certainly open to >>>> suggestions. >>>> [1] >>>> https://github.com/tianocore/edk2/blob/master/OvmfPkg/Library/QemuFwCfgLib/QemuFwCfgDxe.c#L159 >>>> [2] >>>> https://github.com/tianocore/edk2/blob/master/OvmfPkg/Library/QemuFwCfgLib/QemuFwCfgDxe.c#L197 >>>>>> >>>>>> - IoMmuMap() does not allocate pages when BusMasterCommonBuffer is >>>>>> requested, and it allocates pages (bounce buffer) otherwise. >>>>>> >>>> I am trying to wrap my head around how we can support >>>> BusMasterCommonBuffer >>>> when buffer was not allocated by us. Changing the memory encryption mask >>>> in >>>> a page table will not update the contents. Also since the memory >>>> encryption >>>> mask works on PAGE_SIZE hence changing the encryption mask on not our >>>> allocated >>>> buffer could mess things up (e.g if NumberOfBytes is not PAGE_SIZE >>>> aligned). >>> >>> >>> I may be missing something in my understanding. Here is a flow I have in >>> my >>> mind, please correct me. >>> >>> OvmfPkg/VirtIoBlk.c: >>> >>> VirtioBlkInit() >>> .... >>> .... >>> VirtioRingInit >>> Virtio->AllocateSharedPages(RingSize, &Ring->Base) >>> PciIo->AllocatePages(RingSize, &RingAddress) >>> Virtio->MapSharedPages(...,BusMasterCommonBuffer, Ring->Base, >>> RingSize, &RingDeviceAddress) >>> ..... >>> ..... >>> >>> This case is straight forward and we can easily maps. No need for bounce >>> buffering. >>> >>> VirtioBlkReadBlocks(..., BufferSize, Buffer,) >>> ...... >>> ...... >>> SynchronousRequest(..., BufferSize, Buffer) >>> .... >>> Virtio->MapSharedPages(..., BusMasterCommonBuffer, Buffer, >>> BufferSize, &DeviceAddress) >>> VirtioAppendDesc(DeviceAddress, BufferSize, ...) >>> VirtioFlush (...) >>> In the above case, "Buffer" was not allocated by us hence we will not >>> able to change the >>> memory encryption attributes. Am I missing something in the flow ? >>> >> >> >> Common buffer mappings may only be created from buffers that were >> allocated by AllocateBuffer(). In fact, that is its main purpose > > > Yes, that part is well understood. If the buffer was allocated by us (e.g > vring, request/status > structure etc) then those should be mapped as "BusMasterCommonBuffer". > > But I am trying to figure out, how to map a data buffers before issuing a > virtio request. e.g when > VirtioBlkReadBlocks() is called, "Buffer" pointer is not a DMA address hence > we need to map it. > I think it should be mapped using "BusMasterWrite" not > "BusMasterCommonBuffer" before adding into vring. >
If the transfer is strictly unidirectional, then that should work. If the transfer goes both ways, you may need to map/unmap for read and then map/unmap for write _______________________________________________ edk2-devel mailing list [email protected] https://lists.01.org/mailman/listinfo/edk2-devel
