Re: [PATCH kernel v9 09/32] vfio: powerpc/spapr: Rework groups attaching
On Thu, Apr 30, 2015 at 12:29:30PM +1000, Alexey Kardashevskiy wrote: > 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? Sorry, my mistake. I misread the patch. -- 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 pgpDzPis0dnBx.pgp Description: PGP signature
Re: [PATCH kernel v9 09/32] vfio: powerpc/spapr: Rework groups attaching
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(>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(>lock); } Signed-off-by: Alexey Kardashevskiy [aw: for the vfio related changes] Acked-by: Alex Williamson Reviewed-by: David Gibson --- 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(>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(>lock); } -- Alexey -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH kernel v9 09/32] vfio: powerpc/spapr: Rework groups attaching
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 a...@ozlabs.ru [aw: for the vfio related changes] Acked-by: Alex Williamson alex.william...@redhat.com Reviewed-by: David Gibson da...@gibson.dropbear.id.au --- 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 majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH kernel v9 09/32] vfio: powerpc/spapr: Rework groups attaching
On Thu, Apr 30, 2015 at 12:29:30PM +1000, Alexey Kardashevskiy wrote: 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? Sorry, my mistake. I misread the patch. -- 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 pgpDzPis0dnBx.pgp Description: PGP signature
Re: [PATCH kernel v9 09/32] vfio: powerpc/spapr: Rework groups attaching
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. > > Signed-off-by: Alexey Kardashevskiy > [aw: for the vfio related changes] > Acked-by: Alex Williamson > Reviewed-by: David Gibson > --- > 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(>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(>lock); > } > -- 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 pgpkmNSSGSs5i.pgp Description: PGP signature
Re: [PATCH kernel v9 09/32] vfio: powerpc/spapr: Rework groups attaching
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. Signed-off-by: Alexey Kardashevskiy a...@ozlabs.ru [aw: for the vfio related changes] Acked-by: Alex Williamson alex.william...@redhat.com Reviewed-by: David Gibson da...@gibson.dropbear.id.au --- 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); } -- 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 pgpkmNSSGSs5i.pgp Description: PGP signature
[PATCH kernel v9 09/32] vfio: powerpc/spapr: Rework groups attaching
This is to make extended ownership and multiple groups support patches simpler for review. This should cause no behavioural change. Signed-off-by: Alexey Kardashevskiy [aw: for the vfio related changes] Acked-by: Alex Williamson Reviewed-by: David Gibson --- 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(>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(>lock); } -- 2.0.0 -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
[PATCH kernel v9 09/32] vfio: powerpc/spapr: Rework groups attaching
This is to make extended ownership and multiple groups support patches simpler for review. This should cause no behavioural change. Signed-off-by: Alexey Kardashevskiy a...@ozlabs.ru [aw: for the vfio related changes] Acked-by: Alex Williamson alex.william...@redhat.com Reviewed-by: David Gibson da...@gibson.dropbear.id.au --- 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); } -- 2.0.0 -- To unsubscribe from this list: send the line unsubscribe linux-kernel in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/