Skip to contentSkip to conversation

Fix ppc64 build error introduced in v6.6.161

PATCH In review Stephen Brennan 3 participants 4 messages Nov 5 – Nov 10 Stable branch

lore
Share & tools
lore
#0 lore raw

cover Fix ppc64 build error introduced in v6.6.161

Hello,

Drgn builds stable kernels regularly in order to run its test suite against. We encountered an error compiling v6.6.161 for ppc64.

The 6.6.161 update contained "powerpc/iommu: Move pSeries specific functions to pseries/iommu.c" [1]. On ppc64 configurations without CONFIG_IOMMU_API, this breaks the build (details in the patch).

Thank you!
Stephen

https://lore.kernel.org/all/[email protected]/

Stephen Brennan (1):
powerpc/pseries/iommu: Restore CONFIG_IOMMU_API dependency

 arch/powerpc/platforms/pseries/iommu.c | 6 ++++++
 1 file changed, 6 insertions(+)
Signature · 1 line
2.52.0
#1 lore raw

PATCH 1/1 powerpc/pseries/iommu: Restore CONFIG_IOMMU_API dependency

In stable v6.6.161 there is a build error with ppc64 when CONFIG_IOMMU_API is not enabled:

arch/powerpc/platforms/pseries/iommu.c: In function 'iommu_take_ownership':
arch/powerpc/platforms/pseries/iommu.c:69:25: error: 'struct iommu_table_ops' has no member named 'xchg_no_kill'
69 | if (!tbl->it_ops->xchg_no_kill)
| ^~
AS arch/powerpc/lib/copy_mc_64.o
arch/powerpc/platforms/pseries/iommu.c: At top level:
arch/powerpc/platforms/pseries/iommu.c:1787:37: error: 'spapr_tce_table_group_ops' defined but not used [-Werror=unused-variable]
1787 | static struct iommu_table_group_ops spapr_tce_table_group_ops = {
| ^~~~~~~~~~~~~~~~~~~~~~~~~
cc1: all warnings being treated as errors

This is due to stable v6.6.161 commit 1e9dfe15404d ("powerpc/iommu: Move pSeries specific functions to pseries/iommu.c"). It was included as a stable-dep-of commit 8f70caad82e9 ("powerpc/pseries/iommu: IOMMU incorrectly marks MMIO range in DDW").

It was backported without its associated fix: commit af199e6ca29c ("powerpc/pseries/iommu: Define spapr_tce_table_group_ops only with CONFIG_IOMMU_API").

The code was supposed to only build under CONFIG_IOMMU_API, but the new location doesn't have this condition. Unfortunately, the fixing commit comes after a fairly large refactor, and doesn't apply on v6.16.161. Instead, apply the ifdefs where they would belong prior to the refactor.

---
 arch/powerpc/platforms/pseries/iommu.c | 6 ++++++
 1 file changed, 6 insertions(+)
arch/powerpc/platforms/pseries/iommu.c +6 -0
index 6a45e3e86f79b..a79927e870a87 100644
@@ -54,6 +54,7 @@ enum {
 	DDW_EXT_QUERY_OUT_SIZE = 2
 };
 
+#ifdef CONFIG_IOMMU_API
 static int iommu_take_ownership(struct iommu_table *tbl)
 {
 	unsigned long flags, i, sz = (tbl->it_size + 7) >> 3;
@@ -104,6 +105,7 @@ static void iommu_release_ownership(struct iommu_table *tbl)
 		spin_unlock(&tbl->pools[i].lock);
 	spin_unlock_irqrestore(&tbl->large_pool.lock, flags);
 }
+#endif
 
 static struct iommu_table *iommu_pseries_alloc_table(int node)
 {
@@ -118,7 +120,9 @@ static struct iommu_table *iommu_pseries_alloc_table(int node)
 	return tbl;
 }
 
+#ifdef CONFIG_IOMMU_API
 static struct iommu_table_group_ops spapr_tce_table_group_ops;
+#endif
 
 static struct iommu_table_group *iommu_pseries_alloc_group(int node)
 {
@@ -1702,6 +1706,7 @@ static bool iommu_bypass_supported_pSeriesLP(struct pci_dev *pdev, u64 dma_mask)
 	return false;
 }
 
+#ifdef CONFIG_IOMMU_API
 /*
  * A simple iommu_table_group_ops which only allows reusing the existing
  * iommu_table. This handles VFIO for POWER7 or the nested KVM.
@@ -1792,6 +1797,7 @@ static struct iommu_table_group_ops spapr_tce_table_group_ops = {
 	.take_ownership = spapr_tce_take_ownership,
 	.release_ownership = spapr_tce_release_ownership,
 };
+#endif
 
 static int iommu_mem_notifier(struct notifier_block *nb, unsigned long action,
 		void *data)
Signature · 1 line
2.52.0
maintainer
#2 lore raw

Re: [PATCH 6.6.y 1/1] powerpc/pseries/iommu: Restore CONFIG_IOMMU_API dependency

Quoted 1 line

powerpc/pseries/iommu: Restore CONFIG_IOMMU_API dependency

This is a hand-adapted replacement for af199e6ca29c ("powerpc/pseries/iommu:
Define spapr_tce_table_group_ops only with CONFIG_IOMMU_API") rather than a clean cherry-pick, and it has no ack from the powerpc side.

Michael, Madhavan: could one of you review it? I will queue it for 6.6 once you are happy with it.

Signature · 2 lines
Thanks,
Sasha
Acked-by on v1 1/1
#3 lore raw

Re: [PATCH 6.6.y 1/1] powerpc/pseries/iommu: Restore CONFIG_IOMMU_API dependency

In reply to Stephen Brennan
Quoted 19 lines · Stephen Brennan

In stable v6.6.161 there is a build error with ppc64 when CONFIG_IOMMU_API is not enabled:

arch/powerpc/platforms/pseries/iommu.c: In function 'iommu_take_ownership':
arch/powerpc/platforms/pseries/iommu.c:69:25: error: 'struct iommu_table_ops' has no member named 'xchg_no_kill'
69 | if (!tbl->it_ops->xchg_no_kill)
| ^~
AS arch/powerpc/lib/copy_mc_64.o
arch/powerpc/platforms/pseries/iommu.c: At top level:
arch/powerpc/platforms/pseries/iommu.c:1787:37: error: 'spapr_tce_table_group_ops' defined but not used [-Werror=unused-variable]
1787 | static struct iommu_table_group_ops spapr_tce_table_group_ops = {
| ^~~~~~~~~~~~~~~~~~~~~~~~~
cc1: all warnings being treated as errors

This is due to stable v6.6.161 commit 1e9dfe15404d ("powerpc/iommu: Move pSeries specific functions to pseries/iommu.c"). It was included as a stable-dep-of commit 8f70caad82e9 ("powerpc/pseries/iommu: IOMMU incorrectly marks MMIO range in DDW").

Quoted 8 lines

It was backported without its associated fix: commit af199e6ca29c ("powerpc/pseries/iommu: Define spapr_tce_table_group_ops only with CONFIG_IOMMU_API").

The code was supposed to only build under CONFIG_IOMMU_API, but the new location doesn't have this condition. Unfortunately, the fixing commit comes after a fairly large refactor, and doesn't apply on v6.16.161. Instead, apply the ifdefs where they would belong prior to the refactor.

Looks fine to me

Quoted 51 lines
---
  arch/powerpc/platforms/pseries/iommu.c | 6 ++++++
  1 file changed, 6 insertions(+)
arch/powerpc/platforms/pseries/iommu.c +6 -0
index 6a45e3e86f79b..a79927e870a87 100644
@@ -54,6 +54,7 @@ enum {
  	DDW_EXT_QUERY_OUT_SIZE = 2
  };
 
+#ifdef CONFIG_IOMMU_API
  static int iommu_take_ownership(struct iommu_table *tbl)
  {
  	unsigned long flags, i, sz = (tbl->it_size + 7) >> 3;
@@ -104,6 +105,7 @@ static void iommu_release_ownership(struct iommu_table *tbl)
  		spin_unlock(&tbl->pools[i].lock);
  	spin_unlock_irqrestore(&tbl->large_pool.lock, flags);
  }
+#endif
 
  static struct iommu_table *iommu_pseries_alloc_table(int node)
  {
@@ -118,7 +120,9 @@ static struct iommu_table *iommu_pseries_alloc_table(int node)
  	return tbl;
  }
 
+#ifdef CONFIG_IOMMU_API
  static struct iommu_table_group_ops spapr_tce_table_group_ops;
+#endif
 
  static struct iommu_table_group *iommu_pseries_alloc_group(int node)
  {
@@ -1702,6 +1706,7 @@ static bool iommu_bypass_supported_pSeriesLP(struct pci_dev *pdev, u64 dma_mask)
  	return false;
  }
 
+#ifdef CONFIG_IOMMU_API
  /*
   * A simple iommu_table_group_ops which only allows reusing the existing
   * iommu_table. This handles VFIO for POWER7 or the nested KVM.
@@ -1792,6 +1797,7 @@ static struct iommu_table_group_ops spapr_tce_table_group_ops = {
  	.take_ownership = spapr_tce_take_ownership,
  	.release_ownership = spapr_tce_release_ownership,
  };
+#endif
 
  static int iommu_mem_notifier(struct notifier_block *nb, unsigned long action,
  		void *data)