On Wed, Mar 19, 2014 at 01:57:41PM -0600, Alex Williamson wrote:
> On Wed, 2014-03-12 at 16:52 +1100, Alexey Kardashevskiy wrote:
> > From: David Gibson <da...@gibson.dropbear.id.au>
[snip]
> > + if (!memory_region_is_ram(mr)) {
> > + DPRINTF("iommu map to non memory area %"HWADDR_PRIx"\n",
> > + xlat);
> > + return;
> > + }
> > + if (len & iotlb->addr_mask) {
> > + DPRINTF("iommu has granularity incompatible with target AS\n");
>
> Is this possible? Assuming len is initially a power-of-2, would the
> translate function change it? Maybe worth a comment to explain.
translate can absolutely change the length. It will generally
truncate it to the IOMMU page size, in fact.
[snip]
> > + DPRINTF("region_add [iommu] %"HWADDR_PRIx" - %"HWADDR_PRIx"\n",
> > + iova, int128_get64(int128_sub(llend, int128_one())));
> > + /*
> > + * FIXME: We should do some checking to see if the
> > + * capabilities of the host VFIO IOMMU are adequate to model
> > + * the guest IOMMU
> > + *
> > + * FIXME: This assumes that the guest IOMMU is empty of
> > + * mappings at this point - we should either enforce this, or
> > + * loop through existing mappings to map them into VFIO.
> > + *
> > + * FIXME: For VFIO iommu types which have KVM acceleration to
> > + * avoid bouncing all map/unmaps through qemu this way, this
> > + * would be the right place to wire that up (tell the KVM
> > + * device emulation the VFIO iommu handles to use).
> > + */
>
> That's a lot of FIXMEs... The second one in particular looks like it
> needs to expand a bit on why this is likely a valid assumption. The
> last one is more of a TODO than a FIXME.
I think #2 isn't a valid assumption in general. It was true for the
situation I was testing at the time, due to the order of pseries
initialization, so I left it to get a proof of concept reasonably
quickly.
But I think that one's a FIXME that actually needs to be fixed.
[snip]
> > + /*
> > + * FIXME: We assume the one big unmap below is adequate to
> > + * remove any individual page mappings in the IOMMU which
> > + * might have been copied into VFIO. That may not be true for
> > + * all IOMMU types
> > + */
>
> We assume this because the IOVA that gets unmapped is the same
> regardless of whether a guest IOMMU is present?
Uh.. no. This assumption works for a page table based IOMMU where a
big unmap just flattens a large range of IO-PTEs. It might not work
for some kind of extent or TLB based IOMMU, where operations are
expected to exactly match the addresses of map operations.
I don't know if IOMMUs that have trouble with this are a realistic
prospect, but they're at least a theoretical possibility, hence the
comment.
>
> > + }
> > +
> > iova = TARGET_PAGE_ALIGN(section->offset_within_address_space);
> > end = (section->offset_within_address_space +
> > int128_get64(section->size)) &
> > TARGET_PAGE_MASK;
>
>
>
--
David Gibson | I'll have my music baroque, and my code
david AT gibson.dropbear.id.au | minimalist, thank you. NOT _the_ _other_
| _way_ _around_!
http://www.ozlabs.org/~dgibson
pgp49NVs9JrNY.pgp
Description: PGP signature
