On 08/23/17 22:41, Laszlo Ersek wrote: > On 08/23/17 14:22, Brijesh Singh wrote: >> 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. >> >> VirtioRingMap() maps the ring buffer system physical to a bus address. >> When an IOMMU is used for translating the address then bus address can >> start at a different offset from the system physical address. > > (1) The paragraph that you now have as first paragraph above was my > suggestion, so thank you for picking it up. However, the second > paragraph should have been deleted; I suggested the now-first paragraph > as a replacement for the now-second one. > > I wrote, "to keep our references within the virtio device protocol". > VirtioRingMap() is a VirtioLib function, which is a utility layer on top > of the virtio device protocol. So, as I said, VirtioLib patches may > refer to both VirtioLib and the protocol, but protocol patches should > preferably only refer to the protocol, and not VirtioLib. > > VirtioLib --+ > | ^ | > | | | > | +-------+ > | > v > VirtioDeviceProtocol --+ > ^ | > | | > +--------+ > > This is also consistent with the reordering of the patches that I asked > for (and that you implemented well in v3, thank you for it). > > So, apologies if I wasn't clear enough of this -- it's not a big deal at > all, I can remove the second paragraph when I push this. > > Reviewed-by: Laszlo Ersek <[email protected]> > > Thanks! > Laszlo > >> >> - 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 <[email protected]> >> Cc: Ard Biesheuvel <[email protected]> >> Cc: Jordan Justen <[email protected]> >> Cc: Tom Lendacky <[email protected]> >> Cc: Laszlo Ersek <[email protected]> >> Contributed-under: TianoCore Contribution Agreement 1.1 >> Signed-off-by: Brijesh Singh <[email protected]> >> --- >> OvmfPkg/Include/Protocol/VirtioDevice.h | 19 >> +++++++++++++++++-- >> OvmfPkg/Library/VirtioMmioDeviceLib/VirtioMmioDevice.h | 3 ++- >> OvmfPkg/VirtioPciDeviceDxe/VirtioPciDevice.h | 3 ++- >> OvmfPkg/Library/VirtioMmioDeviceLib/VirtioMmioDeviceFunctions.c | 5 ++++- >> OvmfPkg/Virtio10Dxe/Virtio10.c | 5 ++++- >> OvmfPkg/VirtioBlkDxe/VirtioBlk.c | 2 +- >> OvmfPkg/VirtioGpuDxe/Commands.c | 6 +++++- >> OvmfPkg/VirtioNetDxe/SnpInitialize.c | 2 +- >> OvmfPkg/VirtioPciDeviceDxe/VirtioPciFunctions.c | 5 ++++- >> OvmfPkg/VirtioRngDxe/VirtioRng.c | 2 +- >> OvmfPkg/VirtioScsiDxe/VirtioScsi.c | 2 +- >> 11 files changed, 42 insertions(+), 12 deletions(-) >> >> diff --git a/OvmfPkg/Include/Protocol/VirtioDevice.h >> b/OvmfPkg/Include/Protocol/VirtioDevice.h >> index 9a01932958a2..2e3a6d6edf04 100644 >> --- a/OvmfPkg/Include/Protocol/VirtioDevice.h >> +++ b/OvmfPkg/Include/Protocol/VirtioDevice.h >> @@ -156,7 +156,21 @@ EFI_STATUS >> @param[in] This This instance of VIRTIO_DEVICE_PROTOCOL >> >> @param[in] Ring The initialized VRING object to take the >> - addresses from. >> + addresses from. The caller is responsible for >> + ensuring that on input, all Ring->NumPages >> pages, >> + starting at Ring->Base, have been successfully >> + mapped with a single call to >> + This->MapSharedBuffer() for CommonBuffer bus >> + master operation..
(2) I'll also remove one of the periods. Thanks Laszlo _______________________________________________ edk2-devel mailing list [email protected] https://lists.01.org/mailman/listinfo/edk2-devel
