[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: [Qemu-devel] [PATCH v4 15/18] spapr_pci_vfio: Enable multiple groups
From: |
Alexey Kardashevskiy |
Subject: |
Re: [Qemu-devel] [PATCH v4 15/18] spapr_pci_vfio: Enable multiple groups per container |
Date: |
Tue, 17 Feb 2015 11:34:27 +1100 |
User-agent: |
Mozilla/5.0 (X11; Linux i686 on x86_64; rv:31.0) Gecko/20100101 Thunderbird/31.4.0 |
-----BEGIN PGP SIGNED MESSAGE-----
Hash: SHA1
On 02/05/2015 03:19 PM, David Gibson wrote:
> On Thu, Jan 29, 2015 at 08:27:27PM +1100, Alexey Kardashevskiy wrote:
>> This enables multiple IOMMU groups in one VFIO container which
>> means that multiple devices from different groups can share the same
>> IOMMU table and locked pages counting can be done once as there is
>> no need to have several containers for two or more groups.
>>
>> This removes a group id from vfio_container_ioctl(). The kernel
>> support is required for this.
>>
>> This adds a check that there is just one VFIO container per PHB
>> address space.
>>
>> Signed-off-by: Alexey Kardashevskiy <address@hidden> ---
>> hw/ppc/spapr_pci_vfio.c | 9 +++------ hw/vfio/common.c | 33
>> +++++++++++++++------------------ include/hw/vfio/vfio.h | 2 +- 3
>> files changed, 19 insertions(+), 25 deletions(-)
>>
>> diff --git a/hw/ppc/spapr_pci_vfio.c b/hw/ppc/spapr_pci_vfio.c index
>> b20ac90..257181d 100644 --- a/hw/ppc/spapr_pci_vfio.c +++
>> b/hw/ppc/spapr_pci_vfio.c @@ -33,11 +33,10 @@ static int
>> spapr_pci_vfio_ddw_query(sPAPRPHBState *sphb, uint32_t
>> *dma32_window_size, uint64_t *dma64_window_size) { -
>> sPAPRPHBVFIOState *svphb = SPAPR_PCI_VFIO_HOST_BRIDGE(sphb); struct
>> vfio_iommu_spapr_tce_info info = { .argsz = sizeof(info) }; int
>> ret;
>>
>> - ret = vfio_container_ioctl(&sphb->iommu_as,
>> svphb->iommugroupid, + ret =
>> vfio_container_ioctl(&sphb->iommu_as, VFIO_IOMMU_SPAPR_TCE_GET_INFO,
>> &info);
>
>
> Huh.. so vfio_container_ioctl() is actually only used by the
> spapr_tce code. What's it doing living in the common vfio code?
vfio_container_ioctl() converts the address space to a container fd. The
code outside VFIO does not have an idea about these
container/group/device fds.
>
>> if (ret) { return ret; @@ -55,7 +54,6 @@ static int
>> spapr_pci_vfio_ddw_create(sPAPRPHBState *sphb, uint32_t liobn,
>> uint32_t page_shift, uint32_t window_shift, sPAPRTCETable **ptcet)
>> { - sPAPRPHBVFIOState *svphb = SPAPR_PCI_VFIO_HOST_BRIDGE(sphb);
>> struct vfio_iommu_spapr_tce_create create = { .argsz =
>> sizeof(create), .page_shift = page_shift, @@ -65,7 +63,7 @@ static
>> int spapr_pci_vfio_ddw_create(sPAPRPHBState *sphb, uint32_t liobn,
>> }; int ret;
>>
>> - ret = vfio_container_ioctl(&sphb->iommu_as,
>> svphb->iommugroupid, + ret =
>> vfio_container_ioctl(&sphb->iommu_as, VFIO_IOMMU_SPAPR_TCE_CREATE,
>> &create); if (ret) { return ret; @@ -87,7 +85,6 @@ static int
>> spapr_pci_vfio_ddw_create(sPAPRPHBState *sphb, uint32_t liobn,
>>
>> static int spapr_pci_vfio_ddw_remove(sPAPRPHBState *sphb,
>> sPAPRTCETable *tcet) { - sPAPRPHBVFIOState *svphb =
>> SPAPR_PCI_VFIO_HOST_BRIDGE(sphb); struct vfio_iommu_spapr_tce_remove
>> remove = { .argsz = sizeof(remove), .start_addr = tcet->bus_offset
>> @@ -95,7 +92,7 @@ static int spapr_pci_vfio_ddw_remove(sPAPRPHBState
>> *sphb, sPAPRTCETable *tcet) int ret;
>>
>> spapr_pci_ddw_remove(sphb, tcet); - ret =
>> vfio_container_ioctl(&sphb->iommu_as, svphb->iommugroupid, + ret
>> = vfio_container_ioctl(&sphb->iommu_as, VFIO_IOMMU_SPAPR_TCE_REMOVE,
>> &remove);
>>
>> return ret; diff --git a/hw/vfio/common.c b/hw/vfio/common.c index
>> 1cafcf8..a26cbae 100644 --- a/hw/vfio/common.c +++
>> b/hw/vfio/common.c @@ -1011,34 +1011,31 @@ void
>> vfio_put_base_device(VFIODevice *vbasedev) close(vbasedev->fd); }
>>
>> -static int vfio_container_do_ioctl(AddressSpace *as, int32_t
>> groupid, +static int vfio_container_do_ioctl(AddressSpace *as, int
>> req, void *param) { - VFIOGroup *group; VFIOContainer
>> *container; - int ret = -1; + int ret; + VFIOAddressSpace
>> *space;
>>
>> - group = vfio_get_group(groupid, as); - if (!group) { -
>> error_report("vfio: group %d not registered", groupid); -
>> return ret; - } + space = vfio_get_address_space(as); +
>> container = QLIST_FIRST(&space->containers);
>>
>> - container = group->container; - if (group->container) { -
>> ret = ioctl(container->fd, req, param); - if (ret < 0) { -
>> error_report("vfio: failed to ioctl container: ret=%d, %s", -
>> ret, strerror(errno)); - } + if (!container ||
>> QLIST_NEXT(container, next)) { + error_report("vfio: multiple
>> containers per PHB are not - supported");
>
> Shouldn't this be an assert? It's qemu code that sets up the
> containers after all.
The VFIO maintainer does not like asserts and I do not want to do things
which he does not like unless I really have to :)
> Also the error message is not right for the case of !container.
Right, that I will fix.
>> + return -1; }
>>
>> - vfio_put_group(group); + ret = ioctl(container->fd, req,
>> param); + if (ret < 0) { + error_report("vfio: failed to
>> ioctl container: ret=%d, %s", + ret,
>> strerror(errno)); + }
>>
>> return ret; }
>>
>> -int vfio_container_ioctl(AddressSpace *as, int32_t groupid, +int
>> vfio_container_ioctl(AddressSpace *as, int req, void *param) { /* We
>> allow only certain ioctls to the container */ @@ -1054,5 +1051,5 @@
>> int vfio_container_ioctl(AddressSpace *as, int32_t groupid, return
>> -1; }
>>
>> - return vfio_container_do_ioctl(as, groupid, req, param); +
>> return vfio_container_do_ioctl(as, req, param); } diff --git
>> a/include/hw/vfio/vfio.h b/include/hw/vfio/vfio.h index
>> 0b26cd8..76b5744 100644 --- a/include/hw/vfio/vfio.h +++
>> b/include/hw/vfio/vfio.h @@ -3,7 +3,7 @@
>>
>> #include "qemu/typedefs.h"
>>
>> -extern int vfio_container_ioctl(AddressSpace *as, int32_t groupid,
>> +extern int vfio_container_ioctl(AddressSpace *as, int req, void
>> *param);
>>
>> #endif
>
- --
Alexey
-----BEGIN PGP SIGNATURE-----
Version: GnuPG v1
iQIcBAEBAgAGBQJU4oyTAAoJEIYTPdgrwSC5SSUQAJH/KS2rnIXXmyZPnFI9FW+T
X780EzHZ2OLdIMoxqSF2x4tpxL7TLgt+3ZBkqOhmjFhhPnMGEz4KnLRJYG4VAoZx
iS5xfbKtR34Dxyz5flhbdTs1BXkUJli5nhiSiXMbQfgsn0ZG2czvvhee9WZExsAL
0EM1UdxWP4Y0TD03msRWjCfuUmtZnGF3F0iMf3Yv3uuwXvT5pFQWe6Wf0JJNPUDC
X3+wvZeGl+gaX3mGbE0iXL4GiWnK+JqkGUt/CrEdnIVD9h0H6wiLY7s82QEkEsK3
IXRJBfL+TXF/RgKAeCyeVZJzAxbw3PWsb4DKb4U4ByWOmdYxXEq35G99pxZEvPn0
DHobPUNk1RyTrDVL9n5W7lbwcOc81xCA5A3Y3z2NnOaPt8J/wcYg6qigkvhDpMql
PIr+1WHAOf++xUDf+EIXhubGSwqNM7FgUDyrvAEWMEUP+ddz74QN9h0LD7ZB/1lR
svPTE+a/kM2biolzwIPTCdwQXroME7lujB1ynvOEwtiGi5BOjwFTHZ/J7P12iYaZ
px5S8WI07cR7d9cyMFkeYrwxlBwj10i9NzFDP+1JngbzrVmpf15ekj4CFAxMo7gV
ubjcQCvOMoCbiljm5Cg7yn1MjlFkp9ZnNAPKv/TVpw6Co4TgljiAMi+w0DsLXJ8y
DS6iczjry50ZOTpUyWbj
=UkLT
-----END PGP SIGNATURE-----