Re: [PATCH for-5.0 v11 08/20] virtio-iommu: Implement translate

[Jean-Philippe Brucker]

On Mon, Jan 06, 2020 at 12:58:50PM -0500, Peter Xu wrote:
> On Mon, Jan 06, 2020 at 06:06:34PM +0100, Jean-Philippe Brucker wrote:
> > On Fri, Dec 20, 2019 at 11:51:00AM -0500, Peter Xu wrote:
> > > On Fri, Dec 20, 2019 at 05:26:42PM +0100, Jean-Philippe Brucker wrote:
> > > > There is at the virtio transport level: the driver sets status to
> > > > FEATURES_OK once it accepted the feature bits, and to DRIVER_OK once its
> > > > fully operational. The virtio-iommu spec says:
> > > > 
> > > >   If the driver does not accept the VIRTIO_IOMMU_F_BYPASS feature, the
> > > >   device SHOULD NOT let endpoints access the guest-physical address 
> > > > space.
> > > > 
> > > > So before features negotiation, there is no access. Afterwards it 
> > > > depends
> > > > if the VIRTIO_IOMMU_F_BYPASS has been accepted by the driver.
> > > 
> > > Before enabling virtio-iommu device, should we still let the devices
> > > to access the whole system address space?  I believe that's at least
> > > what Intel IOMMUs are doing.  From code-wise, its:
> > > 
> > >     if (likely(s->dmar_enabled)) {
> > >         success = vtd_do_iommu_translate(vtd_as, vtd_as->bus, 
> > > vtd_as->devfn,
> > >                                          addr, flag & IOMMU_WO, &iotlb);
> > >     } else {
> > >         /* DMAR disabled, passthrough, use 4k-page*/
> > >         iotlb.iova = addr & VTD_PAGE_MASK_4K;
> > >         iotlb.translated_addr = addr & VTD_PAGE_MASK_4K;
> > >         iotlb.addr_mask = ~VTD_PAGE_MASK_4K;
> > >         iotlb.perm = IOMMU_RW;
> > >         success = true;
> > >     }
> > > 
> > > From hardware-wise, an IOMMU should be close to transparent if you
> > > never enable it, imho.
> > 
> > For hardware that's not necessarily the best choice. As cited in my
> > previous reply it has been shown to introduce vulnerabilities since
> > malicious devices can DMA during boot, before the OS takes control of the
> > IOMMU. The Arm SMMU allows an implementation to adopt a deny policy by
> > default.
> 
> I see.  But then how to read a sector from the block to at least boot
> an OS if we use a default-deny policy?  Does it still need a mapping
> that is established somehow by someone before hand?

Yes, it looks like EDK II uses IOMMU operations in order to access those
devices on platforms where the IOMMU isn't default-bypass (AMD SEV support
is provided by edk2, and a VT-d driver seems provided by edk2-platforms).
However for OVMF we could just set the bypass feature bit in virtio-iommu
device, which doesn't even requires setting up the virtqueue.

I'm missing a piece of the puzzle for Arm platforms though, because it
looks like Trusted Firmware-A sets up the default-deny policy on reset
even when it wasn't hardwired, but doesn't provide a service to create
SMMUv3 mappings for the bootloader.

Thanks,
Jean