RE: [PATCH] virtio-iommu: Default to bypass during boot

[Tian, Kevin]

> From: Jean-Philippe Brucker <jean-philippe@linaro.org>
> Sent: Friday, February 26, 2021 9:01 PM
> 
> On Fri, Feb 26, 2021 at 08:11:41AM +0000, Tian, Kevin wrote:
> > > From: Qemu-devel <qemu-devel-
> bounces+kevin.tian=intel.com@nongnu.org>
> > > On Behalf Of Jean-Philippe Brucker
> > >
> > > On Sun, Feb 21, 2021 at 06:45:18AM -0500, Michael S. Tsirkin wrote:
> > > > On Thu, Feb 18, 2021 at 11:59:30AM +0100, Jean-Philippe Brucker wrote:
> > > > > Currently the virtio-iommu device must be programmed before it
> allows
> > > > > DMA from any PCI device. This can make the VM entirely unusable
> when
> > > a
> > > > > virtio-iommu driver isn't present, for example in a bootloader that
> > > > > loads the OS from storage.
> > > > >
> > > > > Similarly to the other vIOMMU implementations, default to DMA
> > > bypassing
> > > > > the IOMMU during boot. Add a "boot-bypass" option that lets users
> > > change
> > > > > this behavior.
> > > > >
> > > > > Signed-off-by: Jean-Philippe Brucker <jean-philippe@linaro.org>
> > > >
> > > > wouldn't this be a spec violation?
> > > >
> > > >
> > > > When the device is reset, endpoints are not attached to any domain.
> > > >
> > > > If the VIRTIO_IOMMU_F_BYPASS feature is negotiated, all accesses
> from
> > > > unattached endpoints are allowed and translated by the IOMMU using
> the
> > > > identity function. If the feature is not negotiated, any memory access
> > > > from an unattached endpoint fails. Upon attaching an endpoint in
> > > > bypass mode to a new domain, any memory access from the endpoint
> fails,
> > > > since the domain does not contain any mapping.
> > >
> > > Right, but the device behavior regarding BYPASS is specified as:
> > >
> > >   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 I figured that the spec could be read as "before feature negotiation
> > > it's undefined whether the IOMMU is bypassed or not" and so a device
> > > implementation can choose either (previously discussed at [1]). But it's a
> > > stretch, we should clarify this.
> > >
> > > Also, the OS might want to know whether DMA was bypassing the
> IOMMU
> > > before
> > > the device is initialized. If there are strong security requirements, then
> > > boot-bypass means the system was vulnerable to DMA attacks during
> boot.
> > >
> > > So I'd like to add a new feature bit for this,
> VIRTIO_IOMMU_F_BOOT_BYPASS,
> > > that tells whether DMA bypasses the IOMMU before feature negotiation.
> It's
> > > only an indicator, accepting the feature doesn't change anything. I'll
> > > send the spec change shortly.
> > >
> > > Thanks,
> > > Jean
> > >
> > > [1] https://lore.kernel.org/qemu-
> > > devel/20200109104032.GA937123@myrica/
> > >
> >
> > I wonder why we didn't define the default behavior to bypass (to align with
> > other vIOMMUs) in the first place and then have an option (e.g.
> > VIRTIO_IOMMU_F_NOBYPASS) to override in case a stricter policy is
> required.
> 
> Yes in hindsight the polarity should probably have been reversed. Clearly
> I could have put more thoughts into this. When writing the spec it seemed
> natural in terms of security to disallow accesses until software
> explicitly enables bypass.
> 
> In the linked discussion, which happened after the initial spec was
> accepted, we concluded that QEMU should be boot-bypass by default and
> the
> spec ought to be clarified to explicitly allow this, I just didn't take
> time to work on it until now.
> 
> > In concept when a IOMMU device is uninitialized or reset, it essentially
> means
> > no protection in place thus a bypass behavior.
> 
> Not necessarily, the SMMU leaves that choice to the implementation (but it
> does have the right polarity 0 == bypass). The SMMU_GBPA register defines
> the bypass behavior. Implementations can tie it to 0 or 1 to define the
> default behavior of abort or bypass. Then software sets it to 0 or 1 to
> select the runtime policy. Ideally I'd like a similar mechanism for
> virtio-iommu, because it's sticky across reset - no vulnerability window
> when firmware hands over the IOMMU to the OS.
> 

Thanks. Now I see the background of this patch. 😊