Re: [Qemu-devel] [PATCH] vfio: Align iova also to IOMMU page size

[Alex Williamson <alex.williamson@redhat.com>]

On Thu, 2015-12-03 at 16:33 +0000, Peter Maydell wrote:
> On 3 December 2015 at 16:26, Alex Williamson <alex.williamson@redhat.com> wrote:
> > I feel a lot more comfortable if we limit the scope to MMIO regions of
> > PCI devices.  The problems I brought up before about the device not
> > being able to DMA to a target aligned RAM address are still a
> > possibility that I think we want to catch.  To do that, I think we just
> > need:
> >
> > Object *obj = memory_region_owner(section->mr);
> >
> > if (object_dynamic_cast(obj, "pci-device")) {
> >     /* HOST_PAGE_ALIGN... */
> > } else {
> >     /* TARGET_PAGE_ALIGN... */
> > }
> 
> This looks very odd to me, in two ways: (a) behaving differently
> for PCI passthrough vs other kinds of passthrough,

It's a matter of risk.  If we align an MMIO range out of existence all
we've prevented is peer-to-peer DMA between assigned devices.  Chances
of anyone caring about that are slim to none.  If we align RAM out of
existence, that's a much, much more significant risk that we've just
introduced a data integrity issue for the VM.

>  and (b) caring
> about TARGET_PAGE_ALIGN at all. TARGET_PAGE_ALIGN really isn't
> something vfio should need to care about I think.

But I think we do.  If a RAM address is target page aligned, it could be
a valid DMA target for the device.  If we align it out of existence and
the device is programmed to perform a DMA to that address, the IOMMU
will block it, the VM will not be informed and will continue executing
with invalid data.  The host page alignment is only relevant here if we
wanted to round down, which is probably the more correct thing to do,
but is much more complicated due to the aliasing issue I mentioned in a
previous reply.  To do that we really need a MemoryListener on the
device view of the address map rather than the processor view of the
address map.  Lacking that, we want the IOMMU to fault if when we're
asking it to do mappings below the granularity that it's able to do.
Thanks,

Alex