iommu_sva_domain_alloc() is only called in iommu-sva.c, hence make it
static.
On the other hand, iommu_sva_domain_alloc() should not return NULL anymore
after commit <80af5a452024> ("iommu: Add ops->domain_alloc_sva()"), the
removal of inline code avoids potential confusion.
Fixes: 80af5a452024 ("iommu: Add ops->domain_alloc_sva()")
Signed-off-by: Lu Baolu <[email protected]>
---
include/linux/iommu.h | 8 --------
drivers/iommu/iommu-sva.c | 6 ++++--
2 files changed, 4 insertions(+), 10 deletions(-)
diff --git a/include/linux/iommu.h b/include/linux/iommu.h
index 7bc8dff7cf6d..5cdd3d41b87b 100644
--- a/include/linux/iommu.h
+++ b/include/linux/iommu.h
@@ -1527,8 +1527,6 @@ struct iommu_sva *iommu_sva_bind_device(struct device *dev,
struct mm_struct *mm);
void iommu_sva_unbind_device(struct iommu_sva *handle);
u32 iommu_sva_get_pasid(struct iommu_sva *handle);
-struct iommu_domain *iommu_sva_domain_alloc(struct device *dev,
- struct mm_struct *mm);
#else
static inline struct iommu_sva *
iommu_sva_bind_device(struct device *dev, struct mm_struct *mm)
@@ -1553,12 +1551,6 @@ static inline u32 mm_get_enqcmd_pasid(struct mm_struct *mm)
}
static inline void mm_pasid_drop(struct mm_struct *mm) {}
-
-static inline struct iommu_domain *
-iommu_sva_domain_alloc(struct device *dev, struct mm_struct *mm)
-{
- return NULL;
-}
#endif /* CONFIG_IOMMU_SVA */
#ifdef CONFIG_IOMMU_IOPF
diff --git a/drivers/iommu/iommu-sva.c b/drivers/iommu/iommu-sva.c
index 18a35e798b72..25e581299226 100644
--- a/drivers/iommu/iommu-sva.c
+++ b/drivers/iommu/iommu-sva.c
@@ -10,6 +10,8 @@
#include "iommu-priv.h"
static DEFINE_MUTEX(iommu_sva_lock);
+static struct iommu_domain *iommu_sva_domain_alloc(struct device *dev,
+ struct mm_struct *mm);
/* Allocate a PASID for the mm within range (inclusive) */
static struct iommu_mm_data *iommu_alloc_mm_data(struct mm_struct *mm, struct device *dev)
@@ -277,8 +279,8 @@ static int iommu_sva_iopf_handler(struct iopf_group *group)
return 0;
}
-struct iommu_domain *iommu_sva_domain_alloc(struct device *dev,
- struct mm_struct *mm)
+static struct iommu_domain *iommu_sva_domain_alloc(struct device *dev,
+ struct mm_struct *mm)
{
const struct iommu_ops *ops = dev_iommu_ops(dev);
struct iommu_domain *domain;
--
2.34.1
> From: Lu Baolu <[email protected]>
> Sent: Tuesday, May 28, 2024 12:55 PM
>
> iommu_sva_domain_alloc() is only called in iommu-sva.c, hence make it
> static.
>
> On the other hand, iommu_sva_domain_alloc() should not return NULL
> anymore
> after commit <80af5a452024> ("iommu: Add ops->domain_alloc_sva()"), the
> removal of inline code avoids potential confusion.
>
> Fixes: 80af5a452024 ("iommu: Add ops->domain_alloc_sva()")
> Signed-off-by: Lu Baolu <[email protected]>
Reviewed-by: Kevin Tian <[email protected]>
Hi Lu,
On 5/28/2024 10:24 AM, Lu Baolu wrote:
> iommu_sva_domain_alloc() is only called in iommu-sva.c, hence make it
> static.
>
> On the other hand, iommu_sva_domain_alloc() should not return NULL anymore
> after commit <80af5a452024> ("iommu: Add ops->domain_alloc_sva()"), the
> removal of inline code avoids potential confusion.
>
> Fixes: 80af5a452024 ("iommu: Add ops->domain_alloc_sva()")
> Signed-off-by: Lu Baolu <[email protected]>
> ---
> include/linux/iommu.h | 8 --------
> drivers/iommu/iommu-sva.c | 6 ++++--
> 2 files changed, 4 insertions(+), 10 deletions(-)
>
> diff --git a/include/linux/iommu.h b/include/linux/iommu.h
> index 7bc8dff7cf6d..5cdd3d41b87b 100644
> --- a/include/linux/iommu.h
> +++ b/include/linux/iommu.h
> @@ -1527,8 +1527,6 @@ struct iommu_sva *iommu_sva_bind_device(struct device *dev,
> struct mm_struct *mm);
> void iommu_sva_unbind_device(struct iommu_sva *handle);
> u32 iommu_sva_get_pasid(struct iommu_sva *handle);
> -struct iommu_domain *iommu_sva_domain_alloc(struct device *dev,
> - struct mm_struct *mm);
> #else
> static inline struct iommu_sva *
> iommu_sva_bind_device(struct device *dev, struct mm_struct *mm)
> @@ -1553,12 +1551,6 @@ static inline u32 mm_get_enqcmd_pasid(struct mm_struct *mm)
> }
>
> static inline void mm_pasid_drop(struct mm_struct *mm) {}
> -
> -static inline struct iommu_domain *
> -iommu_sva_domain_alloc(struct device *dev, struct mm_struct *mm)
> -{
> - return NULL;
> -}
> #endif /* CONFIG_IOMMU_SVA */
>
> #ifdef CONFIG_IOMMU_IOPF
> diff --git a/drivers/iommu/iommu-sva.c b/drivers/iommu/iommu-sva.c
> index 18a35e798b72..25e581299226 100644
> --- a/drivers/iommu/iommu-sva.c
> +++ b/drivers/iommu/iommu-sva.c
> @@ -10,6 +10,8 @@
> #include "iommu-priv.h"
>
> static DEFINE_MUTEX(iommu_sva_lock);
> +static struct iommu_domain *iommu_sva_domain_alloc(struct device *dev,
> + struct mm_struct *mm);
If we move move iommu_sva_domain_alloc() before iommu_sva_bind_device() then we
can avoid this forward declaration.
-Vasant
On 5/30/24 4:52 PM, Vasant Hegde wrote:
> On 5/28/2024 10:24 AM, Lu Baolu wrote:
>> iommu_sva_domain_alloc() is only called in iommu-sva.c, hence make it
>> static.
>>
>> On the other hand, iommu_sva_domain_alloc() should not return NULL anymore
>> after commit <80af5a452024> ("iommu: Add ops->domain_alloc_sva()"), the
>> removal of inline code avoids potential confusion.
>>
>> Fixes: 80af5a452024 ("iommu: Add ops->domain_alloc_sva()")
>> Signed-off-by: Lu Baolu<[email protected]>
>> ---
>> include/linux/iommu.h | 8 --------
>> drivers/iommu/iommu-sva.c | 6 ++++--
>> 2 files changed, 4 insertions(+), 10 deletions(-)
>>
>> diff --git a/include/linux/iommu.h b/include/linux/iommu.h
>> index 7bc8dff7cf6d..5cdd3d41b87b 100644
>> --- a/include/linux/iommu.h
>> +++ b/include/linux/iommu.h
>> @@ -1527,8 +1527,6 @@ struct iommu_sva *iommu_sva_bind_device(struct device *dev,
>> struct mm_struct *mm);
>> void iommu_sva_unbind_device(struct iommu_sva *handle);
>> u32 iommu_sva_get_pasid(struct iommu_sva *handle);
>> -struct iommu_domain *iommu_sva_domain_alloc(struct device *dev,
>> - struct mm_struct *mm);
>> #else
>> static inline struct iommu_sva *
>> iommu_sva_bind_device(struct device *dev, struct mm_struct *mm)
>> @@ -1553,12 +1551,6 @@ static inline u32 mm_get_enqcmd_pasid(struct mm_struct *mm)
>> }
>>
>> static inline void mm_pasid_drop(struct mm_struct *mm) {}
>> -
>> -static inline struct iommu_domain *
>> -iommu_sva_domain_alloc(struct device *dev, struct mm_struct *mm)
>> -{
>> - return NULL;
>> -}
>> #endif /* CONFIG_IOMMU_SVA */
>>
>> #ifdef CONFIG_IOMMU_IOPF
>> diff --git a/drivers/iommu/iommu-sva.c b/drivers/iommu/iommu-sva.c
>> index 18a35e798b72..25e581299226 100644
>> --- a/drivers/iommu/iommu-sva.c
>> +++ b/drivers/iommu/iommu-sva.c
>> @@ -10,6 +10,8 @@
>> #include "iommu-priv.h"
>>
>> static DEFINE_MUTEX(iommu_sva_lock);
>> +static struct iommu_domain *iommu_sva_domain_alloc(struct device *dev,
>> + struct mm_struct *mm);
> If we move move iommu_sva_domain_alloc() before iommu_sva_bind_device() then we
> can avoid this forward declaration.
Yeah, but that means moving 144 lines of code. That's why I went with
this approach
Best regards,
baolu
On 5/31/2024 6:29 AM, Baolu Lu wrote:
> On 5/30/24 4:52 PM, Vasant Hegde wrote:
>> On 5/28/2024 10:24 AM, Lu Baolu wrote:
>>> iommu_sva_domain_alloc() is only called in iommu-sva.c, hence make it
>>> static.
>>>
>>> On the other hand, iommu_sva_domain_alloc() should not return NULL anymore
>>> after commit <80af5a452024> ("iommu: Add ops->domain_alloc_sva()"), the
>>> removal of inline code avoids potential confusion.
>>>
>>> Fixes: 80af5a452024 ("iommu: Add ops->domain_alloc_sva()")
>>> Signed-off-by: Lu Baolu<[email protected]>
>>> ---
>>> include/linux/iommu.h | 8 --------
>>> drivers/iommu/iommu-sva.c | 6 ++++--
>>> 2 files changed, 4 insertions(+), 10 deletions(-)
>>>
>>> diff --git a/include/linux/iommu.h b/include/linux/iommu.h
>>> index 7bc8dff7cf6d..5cdd3d41b87b 100644
>>> --- a/include/linux/iommu.h
>>> +++ b/include/linux/iommu.h
>>> @@ -1527,8 +1527,6 @@ struct iommu_sva *iommu_sva_bind_device(struct device
>>> *dev,
>>> struct mm_struct *mm);
>>> void iommu_sva_unbind_device(struct iommu_sva *handle);
>>> u32 iommu_sva_get_pasid(struct iommu_sva *handle);
>>> -struct iommu_domain *iommu_sva_domain_alloc(struct device *dev,
>>> - struct mm_struct *mm);
>>> #else
>>> static inline struct iommu_sva *
>>> iommu_sva_bind_device(struct device *dev, struct mm_struct *mm)
>>> @@ -1553,12 +1551,6 @@ static inline u32 mm_get_enqcmd_pasid(struct mm_struct
>>> *mm)
>>> }
>>> static inline void mm_pasid_drop(struct mm_struct *mm) {}
>>> -
>>> -static inline struct iommu_domain *
>>> -iommu_sva_domain_alloc(struct device *dev, struct mm_struct *mm)
>>> -{
>>> - return NULL;
>>> -}
>>> #endif /* CONFIG_IOMMU_SVA */
>>> #ifdef CONFIG_IOMMU_IOPF
>>> diff --git a/drivers/iommu/iommu-sva.c b/drivers/iommu/iommu-sva.c
>>> index 18a35e798b72..25e581299226 100644
>>> --- a/drivers/iommu/iommu-sva.c
>>> +++ b/drivers/iommu/iommu-sva.c
>>> @@ -10,6 +10,8 @@
>>> #include "iommu-priv.h"
>>> static DEFINE_MUTEX(iommu_sva_lock);
>>> +static struct iommu_domain *iommu_sva_domain_alloc(struct device *dev,
>>> + struct mm_struct *mm);
>> If we move move iommu_sva_domain_alloc() before iommu_sva_bind_device() then we
>> can avoid this forward declaration.
>
> Yeah, but that means moving 144 lines of code. That's why I went with
> this approach
Ah right. I didn't realized we have 'iopf_handler' in iommu_sva_domain_alloc().
-Vasant
On Tue, May 28, 2024 at 12:54:58PM +0800, Lu Baolu wrote:
> iommu_sva_domain_alloc() is only called in iommu-sva.c, hence make it
> static.
>
> On the other hand, iommu_sva_domain_alloc() should not return NULL anymore
> after commit <80af5a452024> ("iommu: Add ops->domain_alloc_sva()"), the
> removal of inline code avoids potential confusion.
>
> Fixes: 80af5a452024 ("iommu: Add ops->domain_alloc_sva()")
> Signed-off-by: Lu Baolu <[email protected]>
> ---
> include/linux/iommu.h | 8 --------
> drivers/iommu/iommu-sva.c | 6 ++++--
> 2 files changed, 4 insertions(+), 10 deletions(-)
Reviewed-by: Jason Gunthorpe <[email protected]>
Jason
On Tue, May 28, 2024 at 12:54:58PM +0800, Lu Baolu wrote:
> iommu_sva_domain_alloc() is only called in iommu-sva.c, hence make it
> static.
>
> On the other hand, iommu_sva_domain_alloc() should not return NULL anymore
> after commit <80af5a452024> ("iommu: Add ops->domain_alloc_sva()"), the
> removal of inline code avoids potential confusion.
>
> Fixes: 80af5a452024 ("iommu: Add ops->domain_alloc_sva()")
> Signed-off-by: Lu Baolu <[email protected]>
> ---
> include/linux/iommu.h | 8 --------
> drivers/iommu/iommu-sva.c | 6 ++++--
> 2 files changed, 4 insertions(+), 10 deletions(-)
Applied, thanks.