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

[Jean-Philippe Brucker <jean-philippe@linaro.org>]

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.

> 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)?

Ideally we should aim at supporting the device in both the BIOS and the
OS. Failing that, there should at least be a way to instantiate a
virtio-iommu device that is blocking by default, that cannot be bypassed
unless the controlling software decides to allow it. Could the bypass
policy be a command-line option to the virtio-iommu device?

[...]
> > 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.

At the moment that per-device behavior can be emulated by sending an
ATTACH request followed by identity MAP. It could be a little more
efficient to add a "bypass" flag to the ATTACH request and avoid setting
up the identity mapping manually, since the device then wouldn't need to
look up the mapping on translation, but I don't know how much it would
improve performance. The device could also cache the fact that the address
space is identity-mapped, for the same result. The domain lookup has to
happen in any case, so you can never get the full iommu-free performance
with these mechanisms.

Thanks,
Jean