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

[Peter Xu <peterx@redhat.com>]

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.

Otherwise I'm confused on how a guest (with virtio-iommu) could boot
with a normal BIOS that does not contain a virtio-iommu driver.  For
example, what if the BIOS needs to read some block sectors (as you
mentioned)?

> > >   - This flag is a must for all virtio-iommu emulation, right?
> > >     (otherwise I can't see how system bootstraps..)
> 
> What do you mean by system bootstrap?

Sorry, I meant when the system boots before the OS.

> 
> One thing I've been wondering, and may be related, is how to handle a
> bootloader that wants to read for example an initrd from a virtio-block
> device that's behind the IOMMU.

My understanding is that virtio devices are special in that they can
use the VIRTIO_F_IOMMU_PLATFORM flag to bypass any vIOMMU (though, I
don't think that'll work when virtio hardwares comes to the
world.. because they can't really bypass the IOMMU hardware).

> Either we allow the device to let any DMA
> bypass the device until FEATURES_OK, which is a source of vulnerabilities
> [1], or we have to implement some support for the virtio-iommu in the
> BIOS. Again the F_BYPASS bit would help for this, since all the BIOS has
> to do is set it on boot. However, F_BYPASS is optional, and more complex
> support is needed for setting up identity mappings.
> 
> [1] See "IOMMU protection against I/O attacks: a vulnerability and a proof
> of concept" by Morgan et al, where a malicious device bypassing the IOMMU
> overwrites the IOMMU configuration as it is being created by the OS.
> Arguably we're not too concerned about malicious devices at the moment,
> but I'm not comfortable relaxing this.
> 
> > >   - Should this flag be gone right after OS starts (otherwise I think
> > >     we still have the issue that any malicious device can be seen as
> > >     in PT mode as default)?  How is that done?
> 
> Yes bypass mode assumes that devices and drivers aren't malicious, and the
> IOMMU is only used for things like assigning devices to guest userspace,
> or having large contiguous DMA buffers.

Yes I agree.  However again when the BYPASS flag was introduced, have
you thought of introducing that flag per-device?  IMHO that could be
better because you have a finer granularity on controlling all these,
so you'll be able to reject malicious devices but at the meantime
grant permission to trusted devices.

Thanks,

-- 
Peter Xu