[v11,2/6] iommu/tegra: Add tegra_dev_iommu_get_stream_id() helper

From: Thierry Reding <treding@nvidia.com>

Access to the internals of struct iommu_fwspec by non-IOMMU drivers is
discouraged. Many drivers for Tegra SoCs, however, need access to their
IOMMU stream IDs so that they can be programmed into various hardware
registers.

Formalize this access into a common helper to make it easier to audit
and maintain.

Signed-off-by: Thierry Reding <treding@nvidia.com>
---
 include/linux/iommu.h | 21 +++++++++++++++++++++
 1 file changed, 21 insertions(+)

On Thu, 17 Nov 2022 at 19:27, Thierry Reding <thierry.reding@gmail.com> wrote:
>
> From: Thierry Reding <treding@nvidia.com>
>
> Access to the internals of struct iommu_fwspec by non-IOMMU drivers is
> discouraged. Many drivers for Tegra SoCs, however, need access to their
> IOMMU stream IDs so that they can be programmed into various hardware
> registers.
>
> Formalize this access into a common helper to make it easier to audit
> and maintain.
>
> Signed-off-by: Thierry Reding <treding@nvidia.com>
> ---
>  include/linux/iommu.h | 21 +++++++++++++++++++++
>  1 file changed, 21 insertions(+)
>
> diff --git a/include/linux/iommu.h b/include/linux/iommu.h
> index dc5e6f5c14fc..f0d72acad7db 100644
> --- a/include/linux/iommu.h
> +++ b/include/linux/iommu.h
> @@ -1186,4 +1186,25 @@ static inline u32 iommu_sva_get_pasid(struct iommu_sva *handle)
>  }
>  #endif /* CONFIG_IOMMU_SVA */
>
> +/*
> + * Newer generations of Tegra SoCs require devices' stream IDs to be directly programmed into
> + * some registers. These are always paired with a Tegra SMMU or ARM SMMU, for which the contents
> + * of the struct iommu_fwspec are known. Use this helper to formalize access to these internals.
> + */
> +#define TEGRA_STREAM_ID_BYPASS 0x7f
> +
> +static inline bool tegra_dev_iommu_get_stream_id(struct device *dev, u32 *stream_id)
> +{
> +#ifdef CONFIG_IOMMU_API
> +       struct iommu_fwspec *fwspec = dev_iommu_fwspec_get(dev);
> +
> +       if (fwspec && fwspec->num_ids == 1) {
> +               *stream_id = fwspec->ids[0] & 0xffff;
> +               return true;
> +       }
> +#endif

It's totally up to iommu maintainers to decide, but I personally would
prefer to replace the below part with a stub function, defined when
CONFIG_IOMMU_API is unset.

> +
> +       return false;
> +}
> +
>  #endif /* __LINUX_IOMMU_H */
> --
> 2.38.1
>

Other than that, both patch1 and patch2 look good to me. I can pick up
the series, after an ack from Robin or Joerg.

Kind regards
Uffe

On Fri, Nov 18, 2022 at 09:08:17AM +0100, Ulf Hansson wrote:
> On Thu, 17 Nov 2022 at 19:27, Thierry Reding <thierry.reding@gmail.com> wrote:
> >
> > From: Thierry Reding <treding@nvidia.com>
> >
> > Access to the internals of struct iommu_fwspec by non-IOMMU drivers is
> > discouraged. Many drivers for Tegra SoCs, however, need access to their
> > IOMMU stream IDs so that they can be programmed into various hardware
> > registers.
> >
> > Formalize this access into a common helper to make it easier to audit
> > and maintain.
> >
> > Signed-off-by: Thierry Reding <treding@nvidia.com>
> > ---
> >  include/linux/iommu.h | 21 +++++++++++++++++++++
> >  1 file changed, 21 insertions(+)
> >
> > diff --git a/include/linux/iommu.h b/include/linux/iommu.h
> > index dc5e6f5c14fc..f0d72acad7db 100644
> > --- a/include/linux/iommu.h
> > +++ b/include/linux/iommu.h
> > @@ -1186,4 +1186,25 @@ static inline u32 iommu_sva_get_pasid(struct iommu_sva *handle)
> >  }
> >  #endif /* CONFIG_IOMMU_SVA */
> >
> > +/*
> > + * Newer generations of Tegra SoCs require devices' stream IDs to be directly programmed into
> > + * some registers. These are always paired with a Tegra SMMU or ARM SMMU, for which the contents
> > + * of the struct iommu_fwspec are known. Use this helper to formalize access to these internals.
> > + */
> > +#define TEGRA_STREAM_ID_BYPASS 0x7f
> > +
> > +static inline bool tegra_dev_iommu_get_stream_id(struct device *dev, u32 *stream_id)
> > +{
> > +#ifdef CONFIG_IOMMU_API
> > +       struct iommu_fwspec *fwspec = dev_iommu_fwspec_get(dev);
> > +
> > +       if (fwspec && fwspec->num_ids == 1) {
> > +               *stream_id = fwspec->ids[0] & 0xffff;
> > +               return true;
> > +       }
> > +#endif
> 
> It's totally up to iommu maintainers to decide, but I personally would
> prefer to replace the below part with a stub function, defined when
> CONFIG_IOMMU_API is unset.

Yeah, that's what I had done originally, but then it felt a bit at odds
with Robin's request to move this into a central location. Ultimately
the result is the same and this has the benefit of standing out a bit
more as unusual.

I don't feel strongly, though, so if Joerg and/or Robin want to see this
split up into an implementation and a stub, I can do that.

Thierry