On 12/01/2019 03:45, Eric Auger wrote:
> In vfio_connect_container() the code that selects the
> iommu type can benefit from helpers such as
> vfio_iommu_get_type() and vfio_init_container(). As
> a result we end up with a switch/case on the iommu type
> that makes the code a little bit more readable and ready
> for addition of new iommu types. Also ioctl's get called
> once per iommu_type.
ioctl(VFIO_SET_IOMMU) is called twice for a reason - on sPAPR, the IOMMU
subdriver is always capable of v1 and v2 but the running platform is not
- powernv supports v2, pseries does not but there is no way telling this
until an IOMMU group is added to a container. So the existing code tries
v2, if that fails, it tries v1:
https://git.qemu.org/?p=qemu.git;a=commitdiff;h=c6e7958eb76ed267f7
>
> Signed-off-by: Eric Auger <eric.au...@redhat.com>
> Reviewed-by: Greg Kurz <gr...@kaod.org>
>
> ---
>
> v2 -> v3:
> - originally submitted in [RFC v2 00/28] vSMMUv3/pSMMUv3
> 2 stage VFIO integration
> - remove "nested only is selected if requested by @force_nested"
> - added Greg's R-b
> ---
> hw/vfio/common.c | 101 ++++++++++++++++++++++++++++++-----------------
> 1 file changed, 64 insertions(+), 37 deletions(-)
>
> diff --git a/hw/vfio/common.c b/hw/vfio/common.c
> index 7c185e5a2e..8535bfe66f 100644
> --- a/hw/vfio/common.c
> +++ b/hw/vfio/common.c
> @@ -1036,12 +1036,57 @@ static void vfio_put_address_space(VFIOAddressSpace
> *space)
> }
> }
>
> +/*
> + * vfio_iommu_get_type - selects the richest iommu_type (v2 first)
> + */
> +static int vfio_iommu_get_type(VFIOContainer *container,
> + Error **errp)
> +{
> + int fd = container->fd;
> +
> + if (ioctl(fd, VFIO_CHECK_EXTENSION, VFIO_TYPE1v2_IOMMU)) {
> + return VFIO_TYPE1v2_IOMMU;
> + } else if (ioctl(fd, VFIO_CHECK_EXTENSION, VFIO_TYPE1_IOMMU)) {
> + return VFIO_TYPE1_IOMMU;
> + } else if (ioctl(fd, VFIO_CHECK_EXTENSION, VFIO_SPAPR_TCE_v2_IOMMU)) {
> + return VFIO_SPAPR_TCE_v2_IOMMU;
> + } else if (ioctl(fd, VFIO_CHECK_EXTENSION, VFIO_SPAPR_TCE_IOMMU)) {
> + return VFIO_SPAPR_TCE_IOMMU;
> + } else {
> + error_setg(errp, "No available IOMMU models");
> + return -EINVAL;
> + }
> +}
> +
> +static int vfio_init_container(VFIOContainer *container, int group_fd,
> + int iommu_type, Error **errp)
> +{
> + int ret;
> +
> + ret = ioctl(group_fd, VFIO_GROUP_SET_CONTAINER, &container->fd);
> + if (ret) {
> + error_setg_errno(errp, errno, "failed to set group container");
> + return -errno;
> + }
> +
> + ret = ioctl(container->fd, VFIO_SET_IOMMU, iommu_type);
> + if (ret) {
> + error_setg_errno(errp, errno, "failed to set iommu for container");
> + return -errno;
> + }
> + container->iommu_type = iommu_type;
> + return 0;
> +}
> +
> static int vfio_connect_container(VFIOGroup *group, AddressSpace *as,
> Error **errp)
> {
> VFIOContainer *container;
> int ret, fd;
> VFIOAddressSpace *space;
> + int iommu_type;
> + bool v2 = false;
> +
>
> space = vfio_get_address_space(as);
>
> @@ -1101,23 +1146,20 @@ static int vfio_connect_container(VFIOGroup *group,
> AddressSpace *as,
> container->fd = fd;
> QLIST_INIT(&container->giommu_list);
> QLIST_INIT(&container->hostwin_list);
> - if (ioctl(fd, VFIO_CHECK_EXTENSION, VFIO_TYPE1_IOMMU) ||
> - ioctl(fd, VFIO_CHECK_EXTENSION, VFIO_TYPE1v2_IOMMU)) {
> - bool v2 = !!ioctl(fd, VFIO_CHECK_EXTENSION, VFIO_TYPE1v2_IOMMU);
> +
> + iommu_type = vfio_iommu_get_type(container, errp);
> + if (iommu_type < 0) {
> + goto free_container_exit;
> + }
> +
> + switch (iommu_type) {
> + case VFIO_TYPE1v2_IOMMU:
> + case VFIO_TYPE1_IOMMU:
> + {
> struct vfio_iommu_type1_info info;
>
> - ret = ioctl(group->fd, VFIO_GROUP_SET_CONTAINER, &fd);
> + ret = vfio_init_container(container, group->fd, iommu_type, errp);
> if (ret) {
> - error_setg_errno(errp, errno, "failed to set group container");
> - ret = -errno;
> - goto free_container_exit;
> - }
> -
> - container->iommu_type = v2 ? VFIO_TYPE1v2_IOMMU : VFIO_TYPE1_IOMMU;
> - ret = ioctl(fd, VFIO_SET_IOMMU, container->iommu_type);
> - if (ret) {
> - error_setg_errno(errp, errno, "failed to set iommu for
> container");
> - ret = -errno;
> goto free_container_exit;
> }
>
> @@ -1137,28 +1179,16 @@ static int vfio_connect_container(VFIOGroup *group,
> AddressSpace *as,
> }
> vfio_host_win_add(container, 0, (hwaddr)-1, info.iova_pgsizes);
> container->pgsizes = info.iova_pgsizes;
> - } else if (ioctl(fd, VFIO_CHECK_EXTENSION, VFIO_SPAPR_TCE_IOMMU) ||
> - ioctl(fd, VFIO_CHECK_EXTENSION, VFIO_SPAPR_TCE_v2_IOMMU)) {
> + break;
> + }
> + case VFIO_SPAPR_TCE_v2_IOMMU:
> + v2 = true;
> + case VFIO_SPAPR_TCE_IOMMU:
> + {
> struct vfio_iommu_spapr_tce_info info;
> - bool v2 = !!ioctl(fd, VFIO_CHECK_EXTENSION, VFIO_SPAPR_TCE_v2_IOMMU);
>
> - ret = ioctl(group->fd, VFIO_GROUP_SET_CONTAINER, &fd);
> + ret = vfio_init_container(container, group->fd, iommu_type, errp);
> if (ret) {
> - error_setg_errno(errp, errno, "failed to set group container");
> - ret = -errno;
> - goto free_container_exit;
> - }
> - container->iommu_type =
> - v2 ? VFIO_SPAPR_TCE_v2_IOMMU : VFIO_SPAPR_TCE_IOMMU;
> - ret = ioctl(fd, VFIO_SET_IOMMU, container->iommu_type);
> - if (ret) {
> - container->iommu_type = VFIO_SPAPR_TCE_IOMMU;
> - v2 = false;
> - ret = ioctl(fd, VFIO_SET_IOMMU, container->iommu_type);
> - }
> - if (ret) {
> - error_setg_errno(errp, errno, "failed to set iommu for
> container");
> - ret = -errno;
> goto free_container_exit;
> }
>
> @@ -1222,10 +1252,7 @@ static int vfio_connect_container(VFIOGroup *group,
> AddressSpace *as,
> info.dma32_window_size - 1,
> 0x1000);
> }
> - } else {
> - error_setg(errp, "No available IOMMU models");
> - ret = -EINVAL;
> - goto free_container_exit;
> + }
> }
>
> vfio_kvm_device_add_group(group);
>
--
Alexey