Re: [Qemu-devel] [PATCH v4 18/18] vfio: Enable in-kernel acceleration via VFIO KVM device

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

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

On Tue, Feb 17, 2015 at 01:36:38PM +1100, Alexey Kardashevskiy wrote:
> -----BEGIN PGP SIGNED MESSAGE-----
> Hash: SHA1
> 
> On 02/05/2015 03:49 PM, David Gibson wrote:
> > On Thu, Jan 29, 2015 at 08:27:30PM +1100, Alexey Kardashevskiy wrote:
> >> TCE hypercalls (H_PUT_TCE, H_PUT_TCE_INDIRECT, H_STUFF_TCE) use a
> >> logical bus number (LIOBN) to identify which TCE table the request
> >> is addressed to. However VFIO kernel driver operates with IOMMU
> >> group IDs and has no idea about which LIOBN corresponds to which
> >> group. If the host kernel supports in-kernel acceleration for TCE
> >> calls, we have to provide the LIOBN to IOMMU mapping information.
> >> 
> >> This makes use of a VFIO KVM device's 
> >> KVM_DEV_VFIO_GROUP_SET_SPAPR_TCE_LIOBN attribute to set the link 
> >> between LIOBN and IOMMU group.
> >> 
> >> The vfio_container_spapr_set_liobn() helper is implemented
> >> completely in vfio.c because kvm_vfio_spapr_tce_liobn needs a group
> >> fd and we do not want to share resources likes that outside vfio.c.
> > 
> > I thought you'd moved away from the idea of in-kernel TCE 
> > acceleration, since big DMA windows made it unnecessary.
> 
> 
> Not entirely. DDW may not be supported by some hardware (like nVidia
> being able to generate only 40bit DMA addresses without hacks). DMA
> memory registering is pretty much about it - when we preregister pages,
> the KVM acceleration becomes lot simpler as it does not have to take care
> of pinning or locked_vm accounting.

Ok, that makes sense.

> But I should have not posted it here anyway, too early :)


> >> Signed-off-by: Alexey Kardashevskiy <aik@ozlabs.ru> --- 
> >> hw/ppc/spapr_iommu.c    |  1 + hw/ppc/spapr_pci_vfio.c | 11
> >> +++++++++++ hw/vfio/common.c        | 33
> >> +++++++++++++++++++++++++++++++++ include/hw/vfio/vfio.h  |  4 ++++ 
> >> 4 files changed, 49 insertions(+)
> >> 
> >> diff --git a/hw/ppc/spapr_iommu.c b/hw/ppc/spapr_iommu.c index
> >> 258f837..3de95d7 100644 --- a/hw/ppc/spapr_iommu.c +++
> >> b/hw/ppc/spapr_iommu.c @@ -142,6 +142,7 @@ static int
> >> spapr_tce_table_realize(DeviceState *dev) if (!tcet->table) { size_t
> >> table_size = tcet->nb_table * sizeof(uint64_t); tcet->table =
> >> g_malloc0(table_size); +        tcet->vfio_accel = false;
> > 
> > This should probably have a qdev prop so that it can be explicitly 
> > disabled on a per-device basis for testing.
> 
> 
> It is a TCE table's property, how can you disable it if it is not created
> via the command line?

Uh, yeah it would have to be a property on VIO devices and on VFIO
host bridges which the propagates the option to the TCE table.

Hrm, that does sound a bit messy now I think about it.

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