| Message ID | 20230602004824.20731-12-vikram.garhwal@amd.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | dynamic node programming using overlay dtbo | expand |
On 02.06.2023 02:48, Vikram Garhwal wrote: > --- a/xen/drivers/passthrough/device_tree.c > +++ b/xen/drivers/passthrough/device_tree.c > @@ -126,6 +126,47 @@ int iommu_release_dt_devices(struct domain *d) > return 0; > } > > +int iommu_remove_dt_device(struct dt_device_node *np) > +{ > + const struct iommu_ops *ops = iommu_get_ops(); > + struct device *dev = dt_to_dev(np); > + int rc; > + > + if ( !ops ) > + return -EOPNOTSUPP; > + > + spin_lock(&dtdevs_lock); > + > + if ( iommu_dt_device_is_assigned_locked(np) ) > + { > + rc = -EBUSY; > + goto fail; > + } > + > + /* > + * The driver which supports generic IOMMU DT bindings must have this > + * callback implemented. > + */ > + if ( !ops->remove_device ) > + { > + rc = -EOPNOTSUPP; > + goto fail; > + } > + > + /* > + * Remove master device from the IOMMU if latter is present and available. > + * The driver is responsible for removing is_protected flag. > + */ > + rc = ops->remove_device(0, dev); > + > + if ( !rc ) > + iommu_fwspec_free(dev); > + > +fail: > + spin_unlock(&dtdevs_lock); > + return rc; Same remark regarding label indentation - please address throughout the series. Jan
Hi, On 02/06/2023 01:48, Vikram Garhwal wrote: > Remove master device from the IOMMU. This will be helpful when removing the > overlay nodes using dynamic programming during run time. > > Signed-off-by: Vikram Garhwal <vikram.garhwal@amd.com> > Reviewed-by: Michal Orzel <michal.orzel@amd.com> > Acked-by: Jan Beulich <jbeulich@suse.com> > --- > xen/drivers/passthrough/device_tree.c | 41 +++++++++++++++++++++++++++ > xen/include/xen/iommu.h | 2 ++ > 2 files changed, 43 insertions(+) > > diff --git a/xen/drivers/passthrough/device_tree.c b/xen/drivers/passthrough/device_tree.c > index 8cc413f867..301a5bcd97 100644 > --- a/xen/drivers/passthrough/device_tree.c > +++ b/xen/drivers/passthrough/device_tree.c > @@ -126,6 +126,47 @@ int iommu_release_dt_devices(struct domain *d) > return 0; > } > > +int iommu_remove_dt_device(struct dt_device_node *np) > +{ > + const struct iommu_ops *ops = iommu_get_ops(); > + struct device *dev = dt_to_dev(np); > + int rc; > + iommu_add_dt_device() checks if the IOMMU is enabled. I think you should do the same here as well and return 0 if it is disabled. > + if ( !ops ) > + return -EOPNOTSUPP; > + > + spin_lock(&dtdevs_lock); > + > + if ( iommu_dt_device_is_assigned_locked(np) ) > + { > + rc = -EBUSY; > + goto fail; > + } > + > + /* > + * The driver which supports generic IOMMU DT bindings must have this > + * callback implemented. > + */ It is not clear to me why you want to mandate remove_device when using the generic IOMMU DT bindings. But if this is really necessary, then I think the comment should be placed on top of the callback definition rather than in the caller. > + if ( !ops->remove_device ) > + { > + rc = -EOPNOTSUPP; > + goto fail; > + } > + > + /* > + * Remove master device from the IOMMU if latter is present and available. > + * The driver is responsible for removing is_protected flag. > + */ > + rc = ops->remove_device(0, dev); > + > + if ( !rc ) > + iommu_fwspec_free(dev); > + > +fail: > + spin_unlock(&dtdevs_lock); > + return rc; > +} > + > int iommu_add_dt_device(struct dt_device_node *np) > { > const struct iommu_ops *ops = iommu_get_ops(); > diff --git a/xen/include/xen/iommu.h b/xen/include/xen/iommu.h > index 405db59971..0d7924821b 100644 > --- a/xen/include/xen/iommu.h > +++ b/xen/include/xen/iommu.h > @@ -229,6 +229,8 @@ int iommu_release_dt_devices(struct domain *d); > */ > int iommu_add_dt_device(struct dt_device_node *np); > > +int iommu_remove_dt_device(struct dt_device_node *np); > + > int iommu_do_dt_domctl(struct xen_domctl *, struct domain *, > XEN_GUEST_HANDLE_PARAM(xen_domctl_t)); > Cheers,
On Mon, Jun 05, 2023 at 08:37:03PM +0100, Julien Grall wrote: > Hi, > > On 02/06/2023 01:48, Vikram Garhwal wrote: > > Remove master device from the IOMMU. This will be helpful when removing the > > overlay nodes using dynamic programming during run time. > > > > Signed-off-by: Vikram Garhwal <vikram.garhwal@amd.com> > > Reviewed-by: Michal Orzel <michal.orzel@amd.com> > > Acked-by: Jan Beulich <jbeulich@suse.com> > > --- > > xen/drivers/passthrough/device_tree.c | 41 +++++++++++++++++++++++++++ > > xen/include/xen/iommu.h | 2 ++ > > 2 files changed, 43 insertions(+) > > > > diff --git a/xen/drivers/passthrough/device_tree.c b/xen/drivers/passthrough/device_tree.c > > index 8cc413f867..301a5bcd97 100644 > > --- a/xen/drivers/passthrough/device_tree.c > > +++ b/xen/drivers/passthrough/device_tree.c > > @@ -126,6 +126,47 @@ int iommu_release_dt_devices(struct domain *d) > > return 0; > > } > > +int iommu_remove_dt_device(struct dt_device_node *np) > > +{ > > + const struct iommu_ops *ops = iommu_get_ops(); > > + struct device *dev = dt_to_dev(np); > > + int rc; > > + > > iommu_add_dt_device() checks if the IOMMU is enabled. I think you should do > the same here as well and return 0 if it is disabled. Added iommu_enabled check in v8. > > > + if ( !ops ) > > + return -EOPNOTSUPP; > > + > > + spin_lock(&dtdevs_lock); > > + > > + if ( iommu_dt_device_is_assigned_locked(np) ) > > + { > > + rc = -EBUSY; > > + goto fail; > > + } > > + > > + /* > > + * The driver which supports generic IOMMU DT bindings must have this > > + * callback implemented. > > + */ > > It is not clear to me why you want to mandate remove_device when using the > generic IOMMU DT bindings. > > But if this is really necessary, then I think the comment should be placed > on top of the callback definition rather than in the caller. Added a comment on top of remove_generic in smmu.c > > > + if ( !ops->remove_device ) > > + { > > + rc = -EOPNOTSUPP; > > + goto fail; > > + } > > + > > + /* > > + * Remove master device from the IOMMU if latter is present and available. > > + * The driver is responsible for removing is_protected flag. > > + */ > > + rc = ops->remove_device(0, dev); > > + > > + if ( !rc ) > > + iommu_fwspec_free(dev); > > + > > +fail: > > + spin_unlock(&dtdevs_lock); > > + return rc; > > +} > > + > > int iommu_add_dt_device(struct dt_device_node *np) > > { > > const struct iommu_ops *ops = iommu_get_ops(); > > diff --git a/xen/include/xen/iommu.h b/xen/include/xen/iommu.h > > index 405db59971..0d7924821b 100644 > > --- a/xen/include/xen/iommu.h > > +++ b/xen/include/xen/iommu.h > > @@ -229,6 +229,8 @@ int iommu_release_dt_devices(struct domain *d); > > */ > > int iommu_add_dt_device(struct dt_device_node *np); > > +int iommu_remove_dt_device(struct dt_device_node *np); > > + > > int iommu_do_dt_domctl(struct xen_domctl *, struct domain *, > > XEN_GUEST_HANDLE_PARAM(xen_domctl_t)); > > Cheers, > > -- > Julien Grall
diff --git a/xen/drivers/passthrough/device_tree.c b/xen/drivers/passthrough/device_tree.c index 8cc413f867..301a5bcd97 100644 --- a/xen/drivers/passthrough/device_tree.c +++ b/xen/drivers/passthrough/device_tree.c @@ -126,6 +126,47 @@ int iommu_release_dt_devices(struct domain *d) return 0; } +int iommu_remove_dt_device(struct dt_device_node *np) +{ + const struct iommu_ops *ops = iommu_get_ops(); + struct device *dev = dt_to_dev(np); + int rc; + + if ( !ops ) + return -EOPNOTSUPP; + + spin_lock(&dtdevs_lock); + + if ( iommu_dt_device_is_assigned_locked(np) ) + { + rc = -EBUSY; + goto fail; + } + + /* + * The driver which supports generic IOMMU DT bindings must have this + * callback implemented. + */ + if ( !ops->remove_device ) + { + rc = -EOPNOTSUPP; + goto fail; + } + + /* + * Remove master device from the IOMMU if latter is present and available. + * The driver is responsible for removing is_protected flag. + */ + rc = ops->remove_device(0, dev); + + if ( !rc ) + iommu_fwspec_free(dev); + +fail: + spin_unlock(&dtdevs_lock); + return rc; +} + int iommu_add_dt_device(struct dt_device_node *np) { const struct iommu_ops *ops = iommu_get_ops(); diff --git a/xen/include/xen/iommu.h b/xen/include/xen/iommu.h index 405db59971..0d7924821b 100644 --- a/xen/include/xen/iommu.h +++ b/xen/include/xen/iommu.h @@ -229,6 +229,8 @@ int iommu_release_dt_devices(struct domain *d); */ int iommu_add_dt_device(struct dt_device_node *np); +int iommu_remove_dt_device(struct dt_device_node *np); + int iommu_do_dt_domctl(struct xen_domctl *, struct domain *, XEN_GUEST_HANDLE_PARAM(xen_domctl_t));