iommu: Always define struct iommu_fwspec

Message ID 20221018105149.820062-1-thierry.reding@gmail.com
State New
Headers
Series iommu: Always define struct iommu_fwspec |

Commit Message

Thierry Reding Oct. 18, 2022, 10:51 a.m. UTC
  From: Thierry Reding <treding@nvidia.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@nvidia.com>
---
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.

 include/linux/iommu.h | 39 +++++++++++++++++++--------------------
 1 file changed, 19 insertions(+), 20 deletions(-)
  

Comments

Ulf Hansson Oct. 20, 2022, 11:32 a.m. UTC | #1
On Tue, 18 Oct 2022 at 12:51, Thierry Reding <thierry.reding@gmail.com> wrote:
>
> From: Thierry Reding <treding@nvidia.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@nvidia.com>

Reviewed-by: Ulf Hansson <ulf.hansson@linaro.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.

>
>  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
  
Thierry Reding Oct. 27, 2022, 9:11 a.m. UTC | #2
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@gmail.com> wrote:
> >
> > From: Thierry Reding <treding@nvidia.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@nvidia.com>
> 
> Reviewed-by: Ulf Hansson <ulf.hansson@linaro.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.

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

Thierry

> 
> >
> >  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
  
Ulf Hansson Oct. 27, 2022, 12:07 p.m. UTC | #3
On Thu, 27 Oct 2022 at 11:11, Thierry Reding <thierry.reding@gmail.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@gmail.com> wrote:
> > >
> > > From: Thierry Reding <treding@nvidia.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@nvidia.com>
> >
> > Reviewed-by: Ulf Hansson <ulf.hansson@linaro.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
  
Joerg Roedel Nov. 3, 2022, 12:59 p.m. UTC | #4
On Thu, Oct 27, 2022 at 11:11:24AM +0200, Thierry Reding wrote:
> Adding Prathamesh for visibility. Another alternative would be to
> prepend this to Prathamesh's series with an Acked-by from Joerg.
> 
> Joerg, any preference on how to move forward with this?

Sorry, missed the discussion until now. Adding this patch to
Prathamesh's series is fine with me. Feel free to add my

Acked-by: Joerg Roedel <jroedel@suse.de>
  

Patch

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 {};