Re: [PATCH kernel v9 09/32] vfio: powerpc/spapr: Rework groups attaching

[Alexey Kardashevskiy]

On 04/29/2015 12:16 PM, David Gibson wrote:
> On Sat, Apr 25, 2015 at 10:14:33PM +1000, Alexey Kardashevskiy wrote:
> > This is to make extended ownership and multiple groups support patches
> > simpler for review.
> >
> > This should cause no behavioural change.
>
> Um.. this doesn't appear to be true.  Previously removing a group from
> an enabled container would fail with EBUSY, now it forces a disable.


This is the original tce_iommu_detach_group() where I cannot find EBUSY you 
are referring to; it did and does enforce disable. What do I miss here?

static void tce_iommu_detach_group(void *iommu_data,
                struct iommu_group *iommu_group)
{
        struct tce_container *container = iommu_data;
        struct iommu_table *tbl = iommu_group_get_iommudata(iommu_group);

        BUG_ON(!tbl);
        mutex_lock(&container->lock);
        if (tbl != container->tbl) {
                pr_warn("tce_vfio: detaching group #%u, expected group is 
#%u\n",
                                iommu_group_id(iommu_group),
                                iommu_group_id(tbl->it_group));
        } else {
                if (container->enabled) {
                        pr_warn("tce_vfio: detaching group #%u from 
enabled container, forcing disable\n",
                                        iommu_group_id(tbl->it_group));
                        tce_iommu_disable(container);
                }

                /* pr_debug("tce_vfio: detaching group #%u from iommu %p\n",
                                iommu_group_id(iommu_group), iommu_group); */
                container->tbl = NULL;
                tce_iommu_clear(container, tbl, tbl->it_offset, tbl->it_size);
                iommu_release_ownership(tbl);
        }
        mutex_unlock(&container->lock);
}



> > Signed-off-by: Alexey Kardashevskiy <aik@xxxxxxxxx>
> > [aw: for the vfio related changes]
> > Acked-by: Alex Williamson <alex.williamson@xxxxxxxxxx>
> > Reviewed-by: David Gibson <david@xxxxxxxxxxxxxxxxxxxxx>
> > ---
> >   drivers/vfio/vfio_iommu_spapr_tce.c | 40 ++++++++++++++++++++++---------------
> >   1 file changed, 24 insertions(+), 16 deletions(-)
> >
> > diff --git a/drivers/vfio/vfio_iommu_spapr_tce.c b/drivers/vfio/vfio_iommu_spapr_tce.c
> > index 115d5e6..0fbe03e 100644
> > --- a/drivers/vfio/vfio_iommu_spapr_tce.c
> > +++ b/drivers/vfio/vfio_iommu_spapr_tce.c
> > @@ -460,16 +460,21 @@ static int tce_iommu_attach_group(void *iommu_data,
> >   				iommu_group_id(container->tbl->it_group),
> >   				iommu_group_id(iommu_group));
> >   		ret = -EBUSY;
> > -	} else if (container->enabled) {
> > +		goto unlock_exit;
> > +	}
> > +
> > +	if (container->enabled) {
> >   		pr_err("tce_vfio: attaching group #%u to enabled container\n",
> >   				iommu_group_id(iommu_group));
> >   		ret = -EBUSY;
> > -	} else {
> > -		ret = iommu_take_ownership(tbl);
> > -		if (!ret)
> > -			container->tbl = tbl;
> > +		goto unlock_exit;
> >   	}
> >
> > +	ret = iommu_take_ownership(tbl);
> > +	if (!ret)
> > +		container->tbl = tbl;
> > +
> > +unlock_exit:
> >   	mutex_unlock(&container->lock);
> >
> >   	return ret;
> > @@ -487,19 +492,22 @@ static void tce_iommu_detach_group(void *iommu_data,
> >   		pr_warn("tce_vfio: detaching group #%u, expected group is #%u\n",
> >   				iommu_group_id(iommu_group),
> >   				iommu_group_id(tbl->it_group));
> > -	} else {
> > -		if (container->enabled) {
> > -			pr_warn("tce_vfio: detaching group #%u from enabled container, forcing disable\n",
> > -					iommu_group_id(tbl->it_group));
> > -			tce_iommu_disable(container);
> > -		}
> > +		goto unlock_exit;
> > +	}
> >
> > -		/* pr_debug("tce_vfio: detaching group #%u from iommu %p\n",
> > -				iommu_group_id(iommu_group), iommu_group); */
> > -		container->tbl = NULL;
> > -		tce_iommu_clear(container, tbl, tbl->it_offset, tbl->it_size);
> > -		iommu_release_ownership(tbl);
> > +	if (container->enabled) {
> > +		pr_warn("tce_vfio: detaching group #%u from enabled container, forcing disable\n",
> > +				iommu_group_id(tbl->it_group));
> > +		tce_iommu_disable(container);
> >   	}
> > +
> > +	/* pr_debug("tce_vfio: detaching group #%u from iommu %p\n",
> > +	   iommu_group_id(iommu_group), iommu_group); */
> > +	container->tbl = NULL;
> > +	tce_iommu_clear(container, tbl, tbl->it_offset, tbl->it_size);
> > +	iommu_release_ownership(tbl);
> > +
> > +unlock_exit:
> >   	mutex_unlock(&container->lock);
> >   }


--
Alexey
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/