Re: [Qemu-devel] [RFC PATCH 09/10] spapr_iommu: Provide a function to switch a TCE table to allowing VFIO

[David Gibson <david@gibson.dropbear.id.au>]

[-- Attachment #1: Type: text/plain, Size: 1796 bytes --]

On Wed, Sep 23, 2015 at 01:24:00PM +0200, Thomas Huth wrote:
> On 17/09/15 18:54, Alex Williamson wrote:
> > On Thu, 2015-09-17 at 23:09 +1000, David Gibson wrote:
> >> Because of the way non-VFIO guest IOMMU operations are KVM accelerated, not
> >> all TCE tables (guest IOMMU contexts) can support VFIO devices.  Currently,
> >> this is decided at creation time.
> >>
> >> To support hotplug of VFIO devices, we need to allow a TCE table which
> >> previously didn't allow VFIO devices to be switched so that it can.  This
> >> patch adds an spapr_tce_need_vfio() function to do this, by reallocating
> >> the table in userspace if necessary.
> >>
> >> Currently this doesn't allow the KVM acceleration to be re-enabled if all
> >> the VFIO devices are removed.  That's an optimization for another time.
> > 
> > 
> > Same comment as previous patch, spapr_tce_need_userspace_table() (or
> > something) makes the code much more self documenting.
> 
> May I also add the using the word "need" in a function name is could be
> a little bit confusing here? It's maybe just me, but if I read
> ..._need_...() I first think of a function that just checks something
> and returns a boolean for yes or no, not of a function that changes
> something.
> Could you maybe use something like "..._change_to_..." instead?

Yeah, I never did much like that name, just couldn't think of a better
one.

Hmm.. how about spapr_tce_set_need_vfio(), taking a bool parameter.  I
won't implement the reverse transition just yet, but that gives us an
obvious place to put it when/if we do.

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

[-- Attachment #2: Type: application/pgp-signature, Size: 819 bytes --]