linux-kernel - Re: [PATCH] iommu: Always define struct iommu

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Date:   Thu, 27 Oct 2022 14:07:51 +0200
From:   Ulf Hansson <ulf.hansson@...aro.org>
To:     Thierry Reding <thierry.reding@...il.com>,
        Prathamesh Shete <pshete@...dia.com>
Cc:     Joerg Roedel <joro@...tes.org>, Will Deacon <will@...nel.org>,
        Robin Murphy <robin.murphy@....com>, iommu@...ts.linux.dev,
        linux-mmc@...r.kernel.org, linux-kernel@...r.kernel.org
Subject: Re: [PATCH] iommu: Always define struct iommu_fwspec

On Thu, 27 Oct 2022 at 11:11, Thierry Reding <thierry.reding@...il.com> wrote:
>
> On Thu, Oct 20, 2022 at 01:32:41PM +0200, Ulf Hansson wrote:
> > On Tue, 18 Oct 2022 at 12:51, Thierry Reding <thierry.reding@...il.com> wrote:
> > >
> > > From: Thierry Reding <treding@...dia.com>
> > >
> > > In order to fully make use of the !IOMMU_API stub functions, make the
> > > struct iommu_fwspec always available so that users of the stubs can keep
> > > using the structure's internals without causing compile failures.
> > >
> > > Signed-off-by: Thierry Reding <treding@...dia.com>
> >
> > Reviewed-by: Ulf Hansson <ulf.hansson@...aro.org>
> >
> > > ---
> > > Hi Joerg,
> > >
> > > this is a rebased patch extracted from an ancient series that never
> > > ended up getting applied:
> > >
> > >         https://lore.kernel.org/all/20191209120005.2254786-3-thierry.reding@gmail.com/
> > >
> > > You had already acked this particular patch, so maybe you can pick this
> > > up. I've seen at least two discussions where this was brought up again,
> > > so I figured it'd be worth sending this out again because it can help
> > > remove a number of #ifdef blocks throughout the kernel.
> >
> > Yes, this would certainly help to improve the code. To me, it looks
> > like the current stub functions, like dev_iommu_fwspec_get() for
> > example, aren't really useful without $subject patch.
> >
> > Note that, I have a pending patch for mmc that would benefit from
> > this. To prevent me from delaying that, an easy way forward, assuming
> > there are no objections of course, would be to send this for 6.1-rc.
>
> Adding Prathamesh for visibility. Another alternative would be to
> prepend this to Prathamesh's series with an Acked-by from Joerg.

Good idea!

I will then be awaiting a new version from Prathamesh's series, that
includes $subject patch too.

>
> Joerg, any preference on how to move forward with this?
>
> Thierry
>

Kind regards
Uffe

> >
> > >
> > >  include/linux/iommu.h | 39 +++++++++++++++++++--------------------
> > >  1 file changed, 19 insertions(+), 20 deletions(-)
> > >
> > > diff --git a/include/linux/iommu.h b/include/linux/iommu.h
> > > index a325532aeab5..e3295c45d18f 100644
> > > --- a/include/linux/iommu.h
> > > +++ b/include/linux/iommu.h
> > > @@ -173,6 +173,25 @@ enum iommu_dev_features {
> > >
> > >  #define IOMMU_PASID_INVALID    (-1U)
> > >
> > > +/**
> > > + * struct iommu_fwspec - per-device IOMMU instance data
> > > + * @ops: ops for this device's IOMMU
> > > + * @iommu_fwnode: firmware handle for this device's IOMMU
> > > + * @flags: IOMMU_FWSPEC_* flags
> > > + * @num_ids: number of associated device IDs
> > > + * @ids: IDs which this device may present to the IOMMU
> > > + */
> > > +struct iommu_fwspec {
> > > +       const struct iommu_ops  *ops;
> > > +       struct fwnode_handle    *iommu_fwnode;
> > > +       u32                     flags;
> > > +       unsigned int            num_ids;
> > > +       u32                     ids[];
> > > +};
> > > +
> > > +/* ATS is supported */
> > > +#define IOMMU_FWSPEC_PCI_RC_ATS                        (1 << 0)
> > > +
> > >  #ifdef CONFIG_IOMMU_API
> > >
> > >  /**
> > > @@ -598,25 +617,6 @@ extern struct iommu_group *generic_device_group(struct device *dev);
> > >  /* FSL-MC device grouping function */
> > >  struct iommu_group *fsl_mc_device_group(struct device *dev);
> > >
> > > -/**
> > > - * struct iommu_fwspec - per-device IOMMU instance data
> > > - * @ops: ops for this device's IOMMU
> > > - * @iommu_fwnode: firmware handle for this device's IOMMU
> > > - * @flags: IOMMU_FWSPEC_* flags
> > > - * @num_ids: number of associated device IDs
> > > - * @ids: IDs which this device may present to the IOMMU
> > > - */
> > > -struct iommu_fwspec {
> > > -       const struct iommu_ops  *ops;
> > > -       struct fwnode_handle    *iommu_fwnode;
> > > -       u32                     flags;
> > > -       unsigned int            num_ids;
> > > -       u32                     ids[];
> > > -};
> > > -
> > > -/* ATS is supported */
> > > -#define IOMMU_FWSPEC_PCI_RC_ATS                        (1 << 0)
> > > -
> > >  /**
> > >   * struct iommu_sva - handle to a device-mm bond
> > >   */
> > > @@ -680,7 +680,6 @@ bool iommu_group_dma_owner_claimed(struct iommu_group *group);
> > >
> > >  struct iommu_ops {};
> > >  struct iommu_group {};
> > > -struct iommu_fwspec {};
> > >  struct iommu_device {};
> > >  struct iommu_fault_param {};
> > >  struct iommu_iotlb_gather {};
> > > --
> > > 2.37.3
> > >
> >
> > Kind regards
> > Uffe