1. Patchew
  2. Xen
  3. View series

[PATCH v2] xen/arm: smmuv3: mark arm_smmu_disable_pasid __maybe_unused

Stewart Hildebrand posted 1 patch 1 year, 5 months ago
Failed in applying to current master (apply log)
There is a newer version of this series
xen/drivers/passthrough/arm/smmu-v3.c | 2 ++
1 file changed, 2 insertions(+)
[PATCH v2] xen/arm: smmuv3: mark arm_smmu_disable_pasid __maybe_unused
Posted by Stewart Hildebrand 1 year, 5 months ago 
When building with clang 12 and CONFIG_ARM_SMMU_V3=y, we observe the
following build error:

drivers/passthrough/arm/smmu-v3.c:1408:20: error: unused function 'arm_smmu_disable_pasid' [-Werror,-Wunused-function]
static inline void arm_smmu_disable_pasid(struct arm_smmu_master *master) { }
                   ^

arm_smmu_disable_pasid is not currently called from anywhere in Xen, but
it is inside a section of code guarded by CONFIG_PCI_ATS, which may be
helpful in the future if the PASID feature is to be implemented. Add the
attribute __maybe_unused to the function.

Signed-off-by: Stewart Hildebrand <stewart.hildebrand@amd.com>
---
v1->v2:
Add __maybe_unused attribute instead of removing
---
 xen/drivers/passthrough/arm/smmu-v3.c | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/xen/drivers/passthrough/arm/smmu-v3.c b/xen/drivers/passthrough/arm/smmu-v3.c
index 9c9f4630090e..0cdc862f96d1 100644
--- a/xen/drivers/passthrough/arm/smmu-v3.c
+++ b/xen/drivers/passthrough/arm/smmu-v3.c
@@ -1376,6 +1376,7 @@ static int arm_smmu_enable_pasid(struct arm_smmu_master *master)
 	return 0;
 }
 
+__maybe_unused
 static void arm_smmu_disable_pasid(struct arm_smmu_master *master)
 {
 	struct pci_dev *pdev;
@@ -1405,6 +1406,7 @@ static inline int arm_smmu_enable_pasid(struct arm_smmu_master *master)
 	return 0;
 }
 
+__maybe_unused
 static inline void arm_smmu_disable_pasid(struct arm_smmu_master *master) { }
 #endif /* CONFIG_PCI_ATS */
 
-- 
2.39.0
Re: [PATCH v2] xen/arm: smmuv3: mark arm_smmu_disable_pasid __maybe_unused
Posted by Julien Grall 1 year, 5 months ago 
Hi Stewart,

I was about to commit this patch when I noticed the placement of the 
attribute doesn't match what we are usually doing in Xen.

On 13/12/2022 18:18, Stewart Hildebrand wrote:
> When building with clang 12 and CONFIG_ARM_SMMU_V3=y, we observe the
> following build error:
> 
> drivers/passthrough/arm/smmu-v3.c:1408:20: error: unused function 'arm_smmu_disable_pasid' [-Werror,-Wunused-function]
> static inline void arm_smmu_disable_pasid(struct arm_smmu_master *master) { }
>                     ^
> 
> arm_smmu_disable_pasid is not currently called from anywhere in Xen, but
> it is inside a section of code guarded by CONFIG_PCI_ATS, which may be
> helpful in the future if the PASID feature is to be implemented. Add the
> attribute __maybe_unused to the function.
> 
> Signed-off-by: Stewart Hildebrand <stewart.hildebrand@amd.com>
> ---
> v1->v2:
> Add __maybe_unused attribute instead of removing
> ---
>   xen/drivers/passthrough/arm/smmu-v3.c | 2 ++
>   1 file changed, 2 insertions(+)
> 
> diff --git a/xen/drivers/passthrough/arm/smmu-v3.c b/xen/drivers/passthrough/arm/smmu-v3.c
> index 9c9f4630090e..0cdc862f96d1 100644
> --- a/xen/drivers/passthrough/arm/smmu-v3.c
> +++ b/xen/drivers/passthrough/arm/smmu-v3.c
> @@ -1376,6 +1376,7 @@ static int arm_smmu_enable_pasid(struct arm_smmu_master *master)
>   	return 0;
>   }
>   
> +__maybe_unused
>   static void arm_smmu_disable_pasid(struct arm_smmu_master *master)

The attribute should be placed after "void". I.e.:

static void __maybe_unused arm_smmu_disable_pasid(...)

>   {
>   	struct pci_dev *pdev;
> @@ -1405,6 +1406,7 @@ static inline int arm_smmu_enable_pasid(struct arm_smmu_master *master)
>   	return 0;
>   }
>   
> +__maybe_unused
>   static inline void arm_smmu_disable_pasid(struct arm_smmu_master *master) { }

Same here.

>   #endif /* CONFIG_PCI_ATS */
>   

Cheers,

-- 
Julien Grall
Re: [PATCH v2] xen/arm: smmuv3: mark arm_smmu_disable_pasid __maybe_unused
Posted by Stewart Hildebrand 1 year, 5 months ago 
On 12/15/22 06:34, Julien Grall wrote:
> Hi Stewart,
> 
> I was about to commit this patch when I noticed the placement of the
> attribute doesn't match what we are usually doing in Xen.
> 
> On 13/12/2022 18:18, Stewart Hildebrand wrote:
>> When building with clang 12 and CONFIG_ARM_SMMU_V3=y, we observe the
>> following build error:
>>
>> drivers/passthrough/arm/smmu-v3.c:1408:20: error: unused function 'arm_smmu_disable_pasid' [-Werror,-Wunused-function]
>> static inline void arm_smmu_disable_pasid(struct arm_smmu_master *master) { }
>>                     ^
>>
>> arm_smmu_disable_pasid is not currently called from anywhere in Xen, but
>> it is inside a section of code guarded by CONFIG_PCI_ATS, which may be
>> helpful in the future if the PASID feature is to be implemented. Add the
>> attribute __maybe_unused to the function.
>>
>> Signed-off-by: Stewart Hildebrand <stewart.hildebrand@amd.com>
>> ---
>> v1->v2:
>> Add __maybe_unused attribute instead of removing
>> ---
>>   xen/drivers/passthrough/arm/smmu-v3.c | 2 ++
>>   1 file changed, 2 insertions(+)
>>
>> diff --git a/xen/drivers/passthrough/arm/smmu-v3.c b/xen/drivers/passthrough/arm/smmu-v3.c
>> index 9c9f4630090e..0cdc862f96d1 100644
>> --- a/xen/drivers/passthrough/arm/smmu-v3.c
>> +++ b/xen/drivers/passthrough/arm/smmu-v3.c
>> @@ -1376,6 +1376,7 @@ static int arm_smmu_enable_pasid(struct arm_smmu_master *master)
>>       return 0;
>>   }
>>
>> +__maybe_unused
>>   static void arm_smmu_disable_pasid(struct arm_smmu_master *master)
> 
> The attribute should be placed after "void". I.e.:
> 
> static void __maybe_unused arm_smmu_disable_pasid(...)

I had initially tried placing it exactly where you suggest in the first draft of v2 of this patch. However, the line would then exceed 72 characters (actual 81 characters):

static void __maybe_unused arm_smmu_disable_pasid(struct arm_smmu_master *master)

So I found myself juggling with how best to wrap it. How about a newline after the __maybe_unused attribute?

static void __maybe_unused
arm_smmu_disable_pasid(struct arm_smmu_master *master)

and similarly for the 2nd occurrence:

static inline void __maybe_unused
arm_smmu_disable_pasid(struct arm_smmu_master *master) { }

There is precedent for this style of wrapping in xen/common/sched/credit2.c.
Re: [PATCH v2] xen/arm: smmuv3: mark arm_smmu_disable_pasid __maybe_unused
Posted by Julien Grall 1 year, 5 months ago 
Hi Stewart,

On 15/12/2022 14:11, Stewart Hildebrand wrote:
> On 12/15/22 06:34, Julien Grall wrote:
>> Hi Stewart,
>>
>> I was about to commit this patch when I noticed the placement of the
>> attribute doesn't match what we are usually doing in Xen.
>>
>> On 13/12/2022 18:18, Stewart Hildebrand wrote:
>>> When building with clang 12 and CONFIG_ARM_SMMU_V3=y, we observe the
>>> following build error:
>>>
>>> drivers/passthrough/arm/smmu-v3.c:1408:20: error: unused function 'arm_smmu_disable_pasid' [-Werror,-Wunused-function]
>>> static inline void arm_smmu_disable_pasid(struct arm_smmu_master *master) { }
>>>                      ^
>>>
>>> arm_smmu_disable_pasid is not currently called from anywhere in Xen, but
>>> it is inside a section of code guarded by CONFIG_PCI_ATS, which may be
>>> helpful in the future if the PASID feature is to be implemented. Add the
>>> attribute __maybe_unused to the function.
>>>
>>> Signed-off-by: Stewart Hildebrand <stewart.hildebrand@amd.com>
>>> ---
>>> v1->v2:
>>> Add __maybe_unused attribute instead of removing
>>> ---
>>>    xen/drivers/passthrough/arm/smmu-v3.c | 2 ++
>>>    1 file changed, 2 insertions(+)
>>>
>>> diff --git a/xen/drivers/passthrough/arm/smmu-v3.c b/xen/drivers/passthrough/arm/smmu-v3.c
>>> index 9c9f4630090e..0cdc862f96d1 100644
>>> --- a/xen/drivers/passthrough/arm/smmu-v3.c
>>> +++ b/xen/drivers/passthrough/arm/smmu-v3.c
>>> @@ -1376,6 +1376,7 @@ static int arm_smmu_enable_pasid(struct arm_smmu_master *master)
>>>        return 0;
>>>    }
>>>
>>> +__maybe_unused
>>>    static void arm_smmu_disable_pasid(struct arm_smmu_master *master)
>>
>> The attribute should be placed after "void". I.e.:
>>
>> static void __maybe_unused arm_smmu_disable_pasid(...)
> 
> I had initially tried placing it exactly where you suggest in the first draft of v2 of this patch. However, the line would then exceed 72 characters (actual 81 characters):

This doesn't change the problem here but the limit is 80 characters per 
line rather than 72.

> 
> static void __maybe_unused arm_smmu_disable_pasid(struct arm_smmu_master *master)
> 
> So I found myself juggling with how best to wrap it. How about a newline after the __maybe_unused attribute?
> 
> static void __maybe_unused
> arm_smmu_disable_pasid(struct arm_smmu_master *master)
> 
> and similarly for the 2nd occurrence:
> 
> static inline void __maybe_unused
> arm_smmu_disable_pasid(struct arm_smmu_master *master) { }
> 
> There is precedent for this style of wrapping in xen/common/sched/credit2.c.

Ah! I didn't realize the line would have been too long. In this case, 
the newline after __maybe_unused is the way to go.

Cheers,

-- 
Julien Grall
Re: [PATCH v2] xen/arm: smmuv3: mark arm_smmu_disable_pasid __maybe_unused
Posted by Stewart Hildebrand 1 year, 5 months ago 
On 12/15/22 09:51, Julien Grall wrote:
> Hi Stewart,
> 
> On 15/12/2022 14:11, Stewart Hildebrand wrote:
>> On 12/15/22 06:34, Julien Grall wrote:
>>> Hi Stewart,
>>>
>>> I was about to commit this patch when I noticed the placement of the
>>> attribute doesn't match what we are usually doing in Xen.
>>>
>>> On 13/12/2022 18:18, Stewart Hildebrand wrote:
>>>> When building with clang 12 and CONFIG_ARM_SMMU_V3=y, we observe the
>>>> following build error:
>>>>
>>>> drivers/passthrough/arm/smmu-v3.c:1408:20: error: unused function 'arm_smmu_disable_pasid' [-Werror,-Wunused-function]
>>>> static inline void arm_smmu_disable_pasid(struct arm_smmu_master *master) { }
>>>>                      ^
>>>>
>>>> arm_smmu_disable_pasid is not currently called from anywhere in Xen, but
>>>> it is inside a section of code guarded by CONFIG_PCI_ATS, which may be
>>>> helpful in the future if the PASID feature is to be implemented. Add the
>>>> attribute __maybe_unused to the function.
>>>>
>>>> Signed-off-by: Stewart Hildebrand <stewart.hildebrand@amd.com>
>>>> ---
>>>> v1->v2:
>>>> Add __maybe_unused attribute instead of removing
>>>> ---
>>>>    xen/drivers/passthrough/arm/smmu-v3.c | 2 ++
>>>>    1 file changed, 2 insertions(+)
>>>>
>>>> diff --git a/xen/drivers/passthrough/arm/smmu-v3.c b/xen/drivers/passthrough/arm/smmu-v3.c
>>>> index 9c9f4630090e..0cdc862f96d1 100644
>>>> --- a/xen/drivers/passthrough/arm/smmu-v3.c
>>>> +++ b/xen/drivers/passthrough/arm/smmu-v3.c
>>>> @@ -1376,6 +1376,7 @@ static int arm_smmu_enable_pasid(struct arm_smmu_master *master)
>>>>        return 0;
>>>>    }
>>>>
>>>> +__maybe_unused
>>>>    static void arm_smmu_disable_pasid(struct arm_smmu_master *master)
>>>
>>> The attribute should be placed after "void". I.e.:
>>>
>>> static void __maybe_unused arm_smmu_disable_pasid(...)
>>
>> I had initially tried placing it exactly where you suggest in the first draft of v2 of this patch. However, the line would then exceed 72 characters (actual 81 characters):
> 
> This doesn't change the problem here but the limit is 80 characters per
> line rather than 72.
> 
>>
>> static void __maybe_unused arm_smmu_disable_pasid(struct arm_smmu_master *master)
>>
>> So I found myself juggling with how best to wrap it. How about a newline after the __maybe_unused attribute?
>>
>> static void __maybe_unused
>> arm_smmu_disable_pasid(struct arm_smmu_master *master)
>>
>> and similarly for the 2nd occurrence:
>>
>> static inline void __maybe_unused
>> arm_smmu_disable_pasid(struct arm_smmu_master *master) { }
>>
>> There is precedent for this style of wrapping in xen/common/sched/credit2.c.
> 
> Ah! I didn't realize the line would have been too long. In this case,
> the newline after __maybe_unused is the way to go.

Ok, I will send a v3 with this change.

Rahul - may I retain your R-b tag in v3?
Re: [PATCH v2] xen/arm: smmuv3: mark arm_smmu_disable_pasid __maybe_unused
Posted by Rahul Singh 1 year, 5 months ago 
Hi Stewart,

> On 15 Dec 2022, at 3:10 pm, Stewart Hildebrand <stewart.hildebrand@amd.com> wrote:
> 
> On 12/15/22 09:51, Julien Grall wrote:
>> Hi Stewart,
>> 
>> On 15/12/2022 14:11, Stewart Hildebrand wrote:
>>> On 12/15/22 06:34, Julien Grall wrote:
>>>> Hi Stewart,
>>>> 
>>>> I was about to commit this patch when I noticed the placement of the
>>>> attribute doesn't match what we are usually doing in Xen.
>>>> 
>>>> On 13/12/2022 18:18, Stewart Hildebrand wrote:
>>>>> When building with clang 12 and CONFIG_ARM_SMMU_V3=y, we observe the
>>>>> following build error:
>>>>> 
>>>>> drivers/passthrough/arm/smmu-v3.c:1408:20: error: unused function 'arm_smmu_disable_pasid' [-Werror,-Wunused-function]
>>>>> static inline void arm_smmu_disable_pasid(struct arm_smmu_master *master) { }
>>>>>                      ^
>>>>> 
>>>>> arm_smmu_disable_pasid is not currently called from anywhere in Xen, but
>>>>> it is inside a section of code guarded by CONFIG_PCI_ATS, which may be
>>>>> helpful in the future if the PASID feature is to be implemented. Add the
>>>>> attribute __maybe_unused to the function.
>>>>> 
>>>>> Signed-off-by: Stewart Hildebrand <stewart.hildebrand@amd.com>
>>>>> ---
>>>>> v1->v2:
>>>>> Add __maybe_unused attribute instead of removing
>>>>> ---
>>>>>    xen/drivers/passthrough/arm/smmu-v3.c | 2 ++
>>>>>    1 file changed, 2 insertions(+)
>>>>> 
>>>>> diff --git a/xen/drivers/passthrough/arm/smmu-v3.c b/xen/drivers/passthrough/arm/smmu-v3.c
>>>>> index 9c9f4630090e..0cdc862f96d1 100644
>>>>> --- a/xen/drivers/passthrough/arm/smmu-v3.c
>>>>> +++ b/xen/drivers/passthrough/arm/smmu-v3.c
>>>>> @@ -1376,6 +1376,7 @@ static int arm_smmu_enable_pasid(struct arm_smmu_master *master)
>>>>>        return 0;
>>>>>    }
>>>>> 
>>>>> +__maybe_unused
>>>>>    static void arm_smmu_disable_pasid(struct arm_smmu_master *master)
>>>> 
>>>> The attribute should be placed after "void". I.e.:
>>>> 
>>>> static void __maybe_unused arm_smmu_disable_pasid(...)
>>> 
>>> I had initially tried placing it exactly where you suggest in the first draft of v2 of this patch. However, the line would then exceed 72 characters (actual 81 characters):
>> 
>> This doesn't change the problem here but the limit is 80 characters per
>> line rather than 72.
>> 
>>> 
>>> static void __maybe_unused arm_smmu_disable_pasid(struct arm_smmu_master *master)
>>> 
>>> So I found myself juggling with how best to wrap it. How about a newline after the __maybe_unused attribute?
>>> 
>>> static void __maybe_unused
>>> arm_smmu_disable_pasid(struct arm_smmu_master *master)
>>> 
>>> and similarly for the 2nd occurrence:
>>> 
>>> static inline void __maybe_unused
>>> arm_smmu_disable_pasid(struct arm_smmu_master *master) { }
>>> 
>>> There is precedent for this style of wrapping in xen/common/sched/credit2.c.
>> 
>> Ah! I didn't realize the line would have been too long. In this case,
>> the newline after __maybe_unused is the way to go.
> 
> Ok, I will send a v3 with this change.
> 
> Rahul - may I retain your R-b tag in v3?

Yes you can retain my R-b.

Regards,
Rahul
Re: [PATCH v2] xen/arm: smmuv3: mark arm_smmu_disable_pasid __maybe_unused
Posted by Rahul Singh 1 year, 5 months ago 
Hi Stewart,

> On 13 Dec 2022, at 6:18 pm, Stewart Hildebrand <stewart.hildebrand@amd.com> wrote:
> 
> When building with clang 12 and CONFIG_ARM_SMMU_V3=y, we observe the
> following build error:
> 
> drivers/passthrough/arm/smmu-v3.c:1408:20: error: unused function 'arm_smmu_disable_pasid' [-Werror,-Wunused-function]
> static inline void arm_smmu_disable_pasid(struct arm_smmu_master *master) { }
>                   ^
> 
> arm_smmu_disable_pasid is not currently called from anywhere in Xen, but
> it is inside a section of code guarded by CONFIG_PCI_ATS, which may be
> helpful in the future if the PASID feature is to be implemented. Add the
> attribute __maybe_unused to the function.
> 
> Signed-off-by: Stewart Hildebrand <stewart.hildebrand@amd.com>

Reviewed-by: Rahul Singh <rahul.singh@arm.com>

Regards,
Rahul

Patchew 2.1-devel-0870f84

© 2016 - 2024 Red Hat, Inc.