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

[Thierry Reding <thierry.reding@gmail.com>]

[-- Attachment #1: Type: text/plain, Size: 2342 bytes --]

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

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 833 bytes --]