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

[Alexey Kardashevskiy]

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

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


>> Signed-off-by: Alexey Kardashevskiy <address@hidden> --- 
>> 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?



>> }
>> 
>> trace_spapr_iommu_new_table(tcet->liobn, tcet, tcet->table,
>> tcet->fd); diff --git a/hw/ppc/spapr_pci_vfio.c
>> b/hw/ppc/spapr_pci_vfio.c index 257181d..2078187 100644 ---
>> a/hw/ppc/spapr_pci_vfio.c +++ b/hw/ppc/spapr_pci_vfio.c @@ -21,6
>> +21,7 @@ #include "hw/pci-host/spapr.h" #include "linux/vfio.h" 
>> #include "hw/vfio/vfio.h" +#include "qemu/error-report.h"
>> 
>> static Property spapr_phb_vfio_properties[] = { 
>> DEFINE_PROP_INT32("iommu", sPAPRPHBVFIOState, iommugroupid, -1), @@
>> -80,6 +81,16 @@ static int spapr_pci_vfio_ddw_create(sPAPRPHBState
>> *sphb, uint32_t liobn, 
>> memory_region_add_subregion(&sphb->iommu_root,
>> (*ptcet)->bus_offset, spapr_tce_get_iommu(*ptcet));
>> 
>> +    if (!(*ptcet)->vfio_accel) { +        return 0; +    } +    ret
>> = vfio_container_spapr_set_liobn(&sphb->iommu_as, +
>> liobn, (*ptcet)->bus_offset); +    if (ret) { +
>> error_report("spapr-vfio: failed to create link to IOMMU"); +
>> ret = 0; +    } + return ret; }
>> 
>> diff --git a/hw/vfio/common.c b/hw/vfio/common.c index
>> a26cbae..ec778d0 100644 --- a/hw/vfio/common.c +++
>> b/hw/vfio/common.c @@ -1053,3 +1053,36 @@ int
>> vfio_container_ioctl(AddressSpace *as,
>> 
>> return vfio_container_do_ioctl(as, req, param); } + +int
>> vfio_container_spapr_set_liobn(AddressSpace *as, +
>> uint64_t liobn, +                                   uint64_t
>> start_addr) +{ +#ifdef CONFIG_KVM +    int ret; +    struct
>> kvm_vfio_spapr_tce_liobn param = { +        .argsz = sizeof(param), 
>> +        .liobn = liobn, +        .start_addr = start_addr +    }; +
>> struct kvm_device_attr attr = { +        .group =
>> KVM_DEV_VFIO_GROUP, +        .attr =
>> KVM_DEV_VFIO_GROUP_SET_SPAPR_TCE_LIOBN, +        .addr =
>> (uint64_t)(unsigned long)&param, +    }; + +    if
>> (vfio_kvm_device_fd < 0) { +        return 0; +    } + +    ret =
>> ioctl(vfio_kvm_device_fd, KVM_SET_DEVICE_ATTR, &attr); +    if (ret)
>> { +        error_report("vfio: failed to setup liobn for a group:
>> %s", +                     strerror(errno)); +    } + +    return
>> ret; +#else +    return 0; +#endif +} diff --git
>> a/include/hw/vfio/vfio.h b/include/hw/vfio/vfio.h index
>> 76b5744..8457933 100644 --- a/include/hw/vfio/vfio.h +++
>> b/include/hw/vfio/vfio.h @@ -6,4 +6,8 @@ extern int
>> vfio_container_ioctl(AddressSpace *as, int req, void *param);
>> 
>> +extern int vfio_container_spapr_set_liobn(AddressSpace *as, +
>> uint64_t liobn, +                                          uint64_t
>> start_addr); + #endif
> 


- -- 
Alexey
-----BEGIN PGP SIGNATURE-----
Version: GnuPG v1

iQIcBAEBAgAGBQJU4qk1AAoJEIYTPdgrwSC5VVAP/08MMoTSLOphkpCn2aIsvUyE
HGjhkkxzALk/E69kdLbsuOLjTngLj0B+CZJX5ArDitmdUVaPBZOJn52MoL+IUyg1
DNZeQH+2J7t+/UZmauHgkxLDWHgtbYZ0LnHOf5HZF/65hsCgb/771z6q0T0T2TJH
F3UhzUBoXmx18OJjsA17l1KQ9QeWGuWjNGcgzZQ5SqqfKRWZUYEwxWLCAXwFQEmC
Xy7JhJ65SgGlCkFNQf1UhoN0N5pxTBUXIj30mtmkfZN5okM68CXmA23tDsulgd+w
d0FTkWZWzegIkLLt/rOL+ILVZqV3pppEgc0ECb+yXtKuzBeaNDQNLXI+TGQVDxEa
tN9olWp8KVYUo5gircmVnM4/6DQe4pca72YvoCYXDe2fwtU5swaG5fL+0qWnn9OA
j0dG3XLJQfQ6XL75P5KQ42zltjOpfhAq5d6tEB2PwlkmyPih8x1iexq/uH/chGKD
h0eRo9xY6fkvW49Gf2jFJvSJ1PhUqIsEmIf4tLPcg5aRqYKWrVqNV57oDtVVLcRD
yewag/FNWk526bWQhH4Kpn1HGqmV3gIbnhGjprvKaqPUIAnyCTxb5dQN3kNTIGDs
rz+XXbJ2g5M3X6opO2OH65aSzCzjvd36JZbqh4jJPP4uctZfBjcDSLYNCl8OIGT8
RFewBFMpyzSV4SV1vJnn
=KtXz
-----END PGP SIGNATURE-----