| Message ID | 20231004145604.1085358-3-stewart.hildebrand@amd.com (mailing list archive) |
|---|---|
| State | Superseded |
| Headers | show |
| Series | SMMU handling for PCIe Passthrough on ARM | expand |
Hi Stewart, On 04/10/2023 15:55, Stewart Hildebrand wrote: > From: Oleksandr Tyshchenko <oleksandr_tyshchenko@epam.com> > > Move code for processing DT IOMMU specifier to a separate helper. > This helper will be re-used for adding PCI devices by the subsequent > patches as we will need exact the same actions for processing > DT PCI-IOMMU specifier. > > While at it introduce NO_IOMMU to avoid magic "1". > > Signed-off-by: Oleksandr Tyshchenko <oleksandr_tyshchenko@epam.com> > Signed-off-by: Stewart Hildebrand <stewart.hildebrand@amd.com> > --- > v4->v5: > * rebase on top of "dynamic node programming using overlay dtbo" series > * move #define NO_IOMMU 1 to header > * s/these/this/ inside comment > > v3->v4: > * make dt_phandle_args *iommu_spec const > * move !ops->add_device check to helper > > v2->v3: > * no change > > v1->v2: > * no change > > downstream->v1: > * trivial rebase > * s/dt_iommu_xlate/iommu_dt_xlate/ > > (cherry picked from commit c26bab0415ca303df86aba1d06ef8edc713734d3 from > the downstream branch poc/pci-passthrough from > https://gitlab.com/xen-project/people/bmarquis/xen-arm-poc.git) > --- > xen/drivers/passthrough/device_tree.c | 48 +++++++++++++++++---------- > xen/include/xen/iommu.h | 2 ++ > 2 files changed, 32 insertions(+), 18 deletions(-) > > diff --git a/xen/drivers/passthrough/device_tree.c b/xen/drivers/passthrough/device_tree.c > index 075fb25a3706..159ace9856c9 100644 > --- a/xen/drivers/passthrough/device_tree.c > +++ b/xen/drivers/passthrough/device_tree.c > @@ -137,6 +137,30 @@ int iommu_release_dt_devices(struct domain *d) > return 0; > } > > +static int iommu_dt_xlate(struct device *dev, > + const struct dt_phandle_args *iommu_spec) > +{ > + const struct iommu_ops *ops = iommu_get_ops(); NIT: Rather than calling iommu_get_ops(), how about passing the ops as a parameter of iommu_dt_xlate()? Reviewed-by: Julien Grall <jgrall@amazon.com> Cheers,
On 10/24/23 14:39, Julien Grall wrote: > Hi Stewart, > > On 04/10/2023 15:55, Stewart Hildebrand wrote: >> From: Oleksandr Tyshchenko <oleksandr_tyshchenko@epam.com> >> >> Move code for processing DT IOMMU specifier to a separate helper. >> This helper will be re-used for adding PCI devices by the subsequent >> patches as we will need exact the same actions for processing >> DT PCI-IOMMU specifier. >> >> While at it introduce NO_IOMMU to avoid magic "1". >> >> Signed-off-by: Oleksandr Tyshchenko <oleksandr_tyshchenko@epam.com> >> Signed-off-by: Stewart Hildebrand <stewart.hildebrand@amd.com> >> --- >> v4->v5: >> * rebase on top of "dynamic node programming using overlay dtbo" series >> * move #define NO_IOMMU 1 to header >> * s/these/this/ inside comment >> >> v3->v4: >> * make dt_phandle_args *iommu_spec const >> * move !ops->add_device check to helper >> >> v2->v3: >> * no change >> >> v1->v2: >> * no change >> >> downstream->v1: >> * trivial rebase >> * s/dt_iommu_xlate/iommu_dt_xlate/ >> >> (cherry picked from commit c26bab0415ca303df86aba1d06ef8edc713734d3 from >> the downstream branch poc/pci-passthrough from >> https://gitlab.com/xen-project/people/bmarquis/xen-arm-poc.git) >> --- >> xen/drivers/passthrough/device_tree.c | 48 +++++++++++++++++---------- >> xen/include/xen/iommu.h | 2 ++ >> 2 files changed, 32 insertions(+), 18 deletions(-) >> >> diff --git a/xen/drivers/passthrough/device_tree.c b/xen/drivers/passthrough/device_tree.c >> index 075fb25a3706..159ace9856c9 100644 >> --- a/xen/drivers/passthrough/device_tree.c >> +++ b/xen/drivers/passthrough/device_tree.c >> @@ -137,6 +137,30 @@ int iommu_release_dt_devices(struct domain *d) >> return 0; >> } >> >> +static int iommu_dt_xlate(struct device *dev, >> + const struct dt_phandle_args *iommu_spec) >> +{ >> + const struct iommu_ops *ops = iommu_get_ops(); > > NIT: Rather than calling iommu_get_ops(), how about passing the ops as a > parameter of iommu_dt_xlate()? Yes, will do. > > Reviewed-by: Julien Grall <jgrall@amazon.com> Thanks! > > Cheers, > > -- > Julien Grall
diff --git a/xen/drivers/passthrough/device_tree.c b/xen/drivers/passthrough/device_tree.c index 075fb25a3706..159ace9856c9 100644 --- a/xen/drivers/passthrough/device_tree.c +++ b/xen/drivers/passthrough/device_tree.c @@ -137,6 +137,30 @@ int iommu_release_dt_devices(struct domain *d) return 0; } +static int iommu_dt_xlate(struct device *dev, + const struct dt_phandle_args *iommu_spec) +{ + const struct iommu_ops *ops = iommu_get_ops(); + int rc; + + if ( !ops->dt_xlate ) + return -EINVAL; + + if ( !dt_device_is_available(iommu_spec->np) ) + return NO_IOMMU; + + rc = iommu_fwspec_init(dev, &iommu_spec->np->dev); + if ( rc ) + return rc; + + /* + * Provide DT IOMMU specifier which describes the IOMMU master + * interfaces of that device (device IDs, etc) to the driver. + * The driver is responsible to decide how to interpret them. + */ + return ops->dt_xlate(dev, iommu_spec); +} + int iommu_remove_dt_device(struct dt_device_node *np) { const struct iommu_ops *ops = iommu_get_ops(); @@ -146,7 +170,7 @@ int iommu_remove_dt_device(struct dt_device_node *np) ASSERT(rw_is_locked(&dt_host_lock)); if ( !iommu_enabled ) - return 1; + return NO_IOMMU; if ( !ops ) return -EOPNOTSUPP; @@ -187,12 +211,12 @@ int iommu_add_dt_device(struct dt_device_node *np) const struct iommu_ops *ops = iommu_get_ops(); struct dt_phandle_args iommu_spec; struct device *dev = dt_to_dev(np); - int rc = 1, index = 0; + int rc = NO_IOMMU, index = 0; ASSERT(system_state < SYS_STATE_active || rw_is_locked(&dt_host_lock)); if ( !iommu_enabled ) - return 1; + return NO_IOMMU; if ( !ops ) return -EINVAL; @@ -215,27 +239,15 @@ int iommu_add_dt_device(struct dt_device_node *np) { /* * The driver which supports generic IOMMU DT bindings must have - * these callback implemented. + * this callback implemented. */ - if ( !ops->add_device || !ops->dt_xlate ) + if ( !ops->add_device ) { rc = -EINVAL; goto fail; } - if ( !dt_device_is_available(iommu_spec.np) ) - break; - - rc = iommu_fwspec_init(dev, &iommu_spec.np->dev); - if ( rc ) - break; - - /* - * Provide DT IOMMU specifier which describes the IOMMU master - * interfaces of that device (device IDs, etc) to the driver. - * The driver is responsible to decide how to interpret them. - */ - rc = ops->dt_xlate(dev, &iommu_spec); + rc = iommu_dt_xlate(dev, &iommu_spec); if ( rc ) break; diff --git a/xen/include/xen/iommu.h b/xen/include/xen/iommu.h index 0e747b0bbc1c..8cd4b9a6bfb2 100644 --- a/xen/include/xen/iommu.h +++ b/xen/include/xen/iommu.h @@ -245,6 +245,8 @@ int iommu_do_dt_domctl(struct xen_domctl *domctl, struct domain *d, */ int iommu_remove_dt_device(struct dt_device_node *np); +#define NO_IOMMU 1 + #endif /* HAS_DEVICE_TREE */ struct page_info;