Linux Virtualization - Jul 2017 - [RFC v1 0/3] Add VIRTIO_F_IOMMU_PLATFORM support

On Fri, Jul 21, 2017 at 06:17:15AM -0500, Brijesh Singh
wrote:
> 
> On 7/20/17 10:24 PM, Jason Wang wrote:
> >
> >
> > On 2017?07?20? 06:09, Brijesh Singh wrote:
> >> I have found that OVMF fails to detect the disk when
iommu_platform
> >> is set from
> >> qemu cli. The failure occurs during the feature bit negotiation.
> >>
> >> Recently, EDKII introduced IOMMU protocol d1fddc4533bf. SEV patch
> >> series introduced
> >> a IoMmu protocol driver f9d129e68a45 to set a DMA access attribute
> >> and methods to
> >> allocate, free, map and unmap the DMA memory for the master bus
devices
> >>
> >> In this patch series, I have tried to enable the IOMMU_PLATFORM
> >> feature for
> >> VirtioBlkDevice. I am sending this as RFC to seek feedback before
I
> >> extend the support
> >> for other Virtio devices. The patch has been tested in SEV guest -
> >> mainly because
> >> IoMmuDxe driver installs the IOMMU protocol for SEV guest only. If
> >> needed, I can
> >> extend the IoMmuDxe driver to install IOMMU protocol for non SEV
guests.
> >>
> >> qemu cli used for testing:
> >>
> >> # $QEMU \
> >>    ...
> >>    -drive file=${HDA_FILE},if=none,id=disk0,format=qcow2 \
> >>    -device
> >>
virtio-blk-pci,drive=disk0,disable-legacy=on,iommu_platform=true,disable-modern=off,scsi=off
> >>    ...
> >>
> >> Cc: Jordan Justen <jordan.l.justen at intel.com>
> >> Cc: Laszlo Ersek <lersek at redhat.com>
> >> Cc: Jason Wang <jasowang at redhat.com>
> >> Cc: Michael S. Tsirkin <mst at redhat.com>
> >> Contributed-under: TianoCore Contribution Agreement 1.0
> >> Signed-off-by: Brijesh Singh <brijesh.singh at amd.com>
> >>
> >> Brijesh Singh (3):
> >>    OvmfPkg/Include/Virtio10: Define VIRTIO_F_IOMMU_PLATFORM
feature bit
> >>    OvmfPkg/VirtioLib: Add IOMMU_PLATFORM support
> >>    OvmfPkg/VirtioBlkDxe: Add VIRITO_F_IOMMU_PLATFORM support
> >
> > Hi, do we need change virtio-scsi driver as well?
> >
> I see that OVMF has the following virtio drivers, we need to update them
> all:
> 
> VirtioBlkDxe
> VirtioGpuDxe
> VirtioNetDxe
> VirtioRngDxe
> VirtioScsiDxe
> 
> I will wait for Laszlo and Jordan's initial feedback before changing
> other drivers.
I'm not sure about the GPU. Cc relevant maintainers -
can virtio GPU work from behind an IOMMU?

> > Thanks
> >
> >>
> >>   OvmfPkg/Library/VirtioLib/VirtioLib.inf      |   1 +
> >>   OvmfPkg/VirtioBlkDxe/VirtioBlk.inf           |   5 +
> >>   OvmfPkg/VirtioGpuDxe/VirtioGpu.inf           |   1 +
> >>   OvmfPkg/VirtioNetDxe/VirtioNet.inf           |   1 +
> >>   OvmfPkg/VirtioRngDxe/VirtioRng.inf           |   1 +
> >>   OvmfPkg/VirtioScsiDxe/VirtioScsi.inf         |   1 +
> >>   OvmfPkg/Include/IndustryStandard/Virtio095.h |   1 +
> >>   OvmfPkg/Include/IndustryStandard/Virtio10.h  |   5 +
> >>   OvmfPkg/Include/Library/VirtioLib.h          |  20 ++++
> >>   OvmfPkg/Library/VirtioLib/VirtioLib.c        |  96
++++++++++++++-
> >>   OvmfPkg/VirtioBlkDxe/VirtioBlk.c             | 125
> >> ++++++++++++++++++--
> >>   11 files changed, 244 insertions(+), 13 deletions(-)
> >>
> >

Hi,
> > I see that OVMF has the following virtio drivers, we need to update
> > them
> > all:
> > 
> > VirtioBlkDxe
> > VirtioGpuDxe
> > VirtioNetDxe
> > VirtioRngDxe
> > VirtioScsiDxe
> > 
> > I will wait for Laszlo and Jordan's initial feedback before
> > changing
> > other drivers.
> 
> I'm not sure about the GPU. Cc relevant maintainers -
> can virtio GPU work from behind an IOMMU?
GPU uses main memory as backing storage for framebuffers, and this is
passed as guest physical address (scatterlist of addresses to be exact)
to the host.

So, I think no, this isn't going to work with the current code.

Should be possible to fix though.  We need to define what "guest
physical address" should be with VIRTIO_F_IOMMU_PLATFORM enabled
(probably guest pci bus address) and add support for proper iommu
lookups.

cheers,
  Gerd