| Message ID | 20221212160031.31590-1-stewart.hildebrand@amd.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | xen/arm: smmuv3: remove unused function | expand |
Hi Stewart, On 12/12/2022 16:00, 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) { } > ^ > > Remove the function. > > Signed-off-by: Stewart Hildebrand <stewart.hildebrand@amd.com> > --- > There is also a definition of arm_smmu_disable_pasid() just above, > guarded by #ifdef CONFIG_PCI_ATS. Should this one be removed too? It > might be nice to keep this definition for ease of backporting patches > from Linux, but if we ever plan on supporting PCI_ATS in Xen this may > need to be re-visited. Given the function is not called at all, I think this is a bit odd to remove the stub but leave the implementation when CONFIG_PCI_ATS is defined. Rahul, do you plan to use it in the PCI passthrough code? If yes, then I would consider to use __maybe_unused. > > This is a candidate for backport to 4.17, 4.16, and possibly 4.15, > although 4.15 has other issues with clang 12. The SMMUv3 is a tech preview feature. So we don't usually backport fixes. I don't see any reason to diverge here. Also, note that 4.15 is in any case only security supported since October 2022. So such patch would not qualify for backport. Cheers,
Hi Julien, > On 12 Dec 2022, at 4:07 pm, Julien Grall <julien@xen.org> wrote: > > Hi Stewart, > > On 12/12/2022 16:00, 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) { } >> ^ >> Remove the function. >> Signed-off-by: Stewart Hildebrand <stewart.hildebrand@amd.com> >> --- >> There is also a definition of arm_smmu_disable_pasid() just above, >> guarded by #ifdef CONFIG_PCI_ATS. Should this one be removed too? It >> might be nice to keep this definition for ease of backporting patches >> from Linux, but if we ever plan on supporting PCI_ATS in Xen this may >> need to be re-visited. > > Given the function is not called at all, I think this is a bit odd to remove the stub but leave the implementation when CONFIG_PCI_ATS is defined. > > Rahul, do you plan to use it in the PCI passthrough code? If yes, then I would consider to use __maybe_unused. No, this function will not be used in PCI passthrough code, but when we merged the SMMUv3 code from Linux at that time we decided to have this code and gate with CONFIG_PCI_ATS so that in the future if someone wants to implement the PASID feature will use these functions. I also agree with Julien we would consider using __maybe_unused. Regards, Rahul
On 12/12/22 11:24, Rahul Singh wrote: > Hi Julien, > >> On 12 Dec 2022, at 4:07 pm, Julien Grall <julien@xen.org> wrote: >> >> Hi Stewart, >> >> On 12/12/2022 16:00, 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) { } >>> ^ >>> Remove the function. >>> Signed-off-by: Stewart Hildebrand <stewart.hildebrand@amd.com> >>> --- >>> There is also a definition of arm_smmu_disable_pasid() just above, >>> guarded by #ifdef CONFIG_PCI_ATS. Should this one be removed too? It >>> might be nice to keep this definition for ease of backporting patches >>> from Linux, but if we ever plan on supporting PCI_ATS in Xen this may >>> need to be re-visited. >> >> Given the function is not called at all, I think this is a bit odd to remove the stub but leave the implementation when CONFIG_PCI_ATS is defined. >> >> Rahul, do you plan to use it in the PCI passthrough code? If yes, then I would consider to use __maybe_unused. > > No, this function will not be used in PCI passthrough code, but when we merged the SMMUv3 code from Linux at that time we > decided to have this code and gate with CONFIG_PCI_ATS so that in the future if someone wants to implement the PASID feature > will use these functions. > > I also agree with Julien we would consider using __maybe_unused. OK, I will send a v2 with the __maybe_unused attribute (in both locations), in case PASID is to be implemented in the future.
diff --git a/xen/drivers/passthrough/arm/smmu-v3.c b/xen/drivers/passthrough/arm/smmu-v3.c index 9c9f4630090e..f54d85cb4b70 100644 --- a/xen/drivers/passthrough/arm/smmu-v3.c +++ b/xen/drivers/passthrough/arm/smmu-v3.c @@ -1404,8 +1404,6 @@ static inline int arm_smmu_enable_pasid(struct arm_smmu_master *master) { return 0; } - -static inline void arm_smmu_disable_pasid(struct arm_smmu_master *master) { } #endif /* CONFIG_PCI_ATS */ static void arm_smmu_detach_dev(struct arm_smmu_master *master)
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) { } ^ Remove the function. Signed-off-by: Stewart Hildebrand <stewart.hildebrand@amd.com> --- There is also a definition of arm_smmu_disable_pasid() just above, guarded by #ifdef CONFIG_PCI_ATS. Should this one be removed too? It might be nice to keep this definition for ease of backporting patches from Linux, but if we ever plan on supporting PCI_ATS in Xen this may need to be re-visited. This is a candidate for backport to 4.17, 4.16, and possibly 4.15, although 4.15 has other issues with clang 12. --- xen/drivers/passthrough/arm/smmu-v3.c | 2 -- 1 file changed, 2 deletions(-)