Bare unreachable() should be avoided as it generates undefined behavior,
e.g. falling through to the next function.  Use BUG() instead so the
error is defined.

Fixes the following warnings:

  drivers/iommu/dma-iommu.o: warning: objtool: iommu_dma_sw_msi+0x92: can't find jump dest instruction at .text+0x54d5
  vmlinux.o: warning: objtool: iommu_dma_get_msi_page() falls through to next function __iommu_dma_unmap()

Reported-by: Randy Dunlap <rdunlap@infradead.org>
Closes: https://lore.kernel.org/314f8809-cd59-479b-97d7-49356bf1c8d1@infradead.org
Reported-by: Paul E. McKenney <paulmck@kernel.org>
Closes: https://lore.kernel.org/5dd1f35e-8ece-43b7-ad6d-86d02d2718f6@paulmck-laptop
Fixes: 6aa63a4ec947 ("iommu: Sort out domain user data")
Signed-off-by: Josh Poimboeuf <jpoimboe@kernel.org>
---
 drivers/iommu/dma-iommu.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/drivers/iommu/dma-iommu.c b/drivers/iommu/dma-iommu.c
index 7b2734de2ba9..6054d0ab8023 100644
--- a/drivers/iommu/dma-iommu.c
+++ b/drivers/iommu/dma-iommu.c
@@ -1753,7 +1753,7 @@ static size_t cookie_msi_granule(const struct iommu_domain *domain)
 	case IOMMU_COOKIE_DMA_MSI:
 		return PAGE_SIZE;
 	default:
-		unreachable();
+		BUG();
 	};
 }
 
@@ -1765,7 +1765,7 @@ static struct list_head *cookie_msi_pages(const struct iommu_domain *domain)
 	case IOMMU_COOKIE_DMA_MSI:
 		return &domain->msi_cookie->msi_page_list;
 	default:
-		unreachable();
+		BUG();
 	};
 }
 
-- 
2.48.1

On Wed, Mar 26, 2025 at 10:28:46PM -0700, Josh Poimboeuf wrote:
> Bare unreachable() should be avoided as it generates undefined behavior,
> e.g. falling through to the next function.  Use BUG() instead so the
> error is defined.
> 
> Fixes the following warnings:
> 
>   drivers/iommu/dma-iommu.o: warning: objtool: iommu_dma_sw_msi+0x92: can't find jump dest instruction at .text+0x54d5
>   vmlinux.o: warning: objtool: iommu_dma_get_msi_page() falls through to next function __iommu_dma_unmap()
> 
> Reported-by: Randy Dunlap <rdunlap@infradead.org>
> Closes: https://lore.kernel.org/314f8809-cd59-479b-97d7-49356bf1c8d1@infradead.org
> Reported-by: Paul E. McKenney <paulmck@kernel.org>
> Closes: https://lore.kernel.org/5dd1f35e-8ece-43b7-ad6d-86d02d2718f6@paulmck-laptop
> Fixes: 6aa63a4ec947 ("iommu: Sort out domain user data")
> Signed-off-by: Josh Poimboeuf <jpoimboe@kernel.org>

Thank you for the quick response!

Tested-by: Paul E. McKenney <paulmck@kernel.org>

> ---
>  drivers/iommu/dma-iommu.c | 4 ++--
>  1 file changed, 2 insertions(+), 2 deletions(-)
> 
> diff --git a/drivers/iommu/dma-iommu.c b/drivers/iommu/dma-iommu.c
> index 7b2734de2ba9..6054d0ab8023 100644
> --- a/drivers/iommu/dma-iommu.c
> +++ b/drivers/iommu/dma-iommu.c
> @@ -1753,7 +1753,7 @@ static size_t cookie_msi_granule(const struct iommu_domain *domain)
>  	case IOMMU_COOKIE_DMA_MSI:
>  		return PAGE_SIZE;
>  	default:
> -		unreachable();
> +		BUG();
>  	};
>  }
>  
> @@ -1765,7 +1765,7 @@ static struct list_head *cookie_msi_pages(const struct iommu_domain *domain)
>  	case IOMMU_COOKIE_DMA_MSI:
>  		return &domain->msi_cookie->msi_page_list;
>  	default:
> -		unreachable();
> +		BUG();
>  	};
>  }
>  
> -- 
> 2.48.1
>

On Wed, Mar 26, 2025 at 10:28:46PM -0700, Josh Poimboeuf wrote:
> Bare unreachable() should be avoided as it generates undefined behavior,
> e.g. falling through to the next function.  Use BUG() instead so the
> error is defined.
> 
> Fixes the following warnings:
> 
>   drivers/iommu/dma-iommu.o: warning: objtool: iommu_dma_sw_msi+0x92: can't find jump dest instruction at .text+0x54d5
>   vmlinux.o: warning: objtool: iommu_dma_get_msi_page() falls through to next function __iommu_dma_unmap()
> 
> Reported-by: Randy Dunlap <rdunlap@infradead.org>
> Closes: https://lore.kernel.org/314f8809-cd59-479b-97d7-49356bf1c8d1@infradead.org
> Reported-by: Paul E. McKenney <paulmck@kernel.org>
> Closes: https://lore.kernel.org/5dd1f35e-8ece-43b7-ad6d-86d02d2718f6@paulmck-laptop
> Fixes: 6aa63a4ec947 ("iommu: Sort out domain user data")
> Signed-off-by: Josh Poimboeuf <jpoimboe@kernel.org>
> ---
>  drivers/iommu/dma-iommu.c | 4 ++--
>  1 file changed, 2 insertions(+), 2 deletions(-)

The offending patch is in the iommufd tree, so applied thanks for the
quick response

Jason

On Wed, Mar 26, 2025 at 10:28:46PM -0700, Josh Poimboeuf wrote:
> Bare unreachable() should be avoided as it generates undefined behavior,
> e.g. falling through to the next function.  Use BUG() instead so the
> error is defined.

Right; I did a pass like this a while ago and thought I'd removed all
unreachable() abuse.

Compilers see this as a clue to just stop code-gen. Very bad behaviour
if you ever actually get there. BUG() at the very least stops the kernel
dead, instead of continuing to run random code that comes after.

On Thu, Mar 27, 2025 at 01:37:18PM +0100, Peter Zijlstra wrote:
> On Wed, Mar 26, 2025 at 10:28:46PM -0700, Josh Poimboeuf wrote:
> > Bare unreachable() should be avoided as it generates undefined behavior,
> > e.g. falling through to the next function.  Use BUG() instead so the
> > error is defined.
> 
> Right; I did a pass like this a while ago and thought I'd removed all
> unreachable() abuse.

Any reason not to just "#define unreachable() BUG()" and convert UD2 and
similar to use __builtin_unreachable()?

-- 
Josh

On Tue, Apr 01, 2025 at 05:50:32PM -0700, Josh Poimboeuf wrote:
> On Thu, Mar 27, 2025 at 01:37:18PM +0100, Peter Zijlstra wrote:
> > On Wed, Mar 26, 2025 at 10:28:46PM -0700, Josh Poimboeuf wrote:
> > > Bare unreachable() should be avoided as it generates undefined behavior,
> > > e.g. falling through to the next function.  Use BUG() instead so the
> > > error is defined.
> > 
> > Right; I did a pass like this a while ago and thought I'd removed all
> > unreachable() abuse.
> 
> Any reason not to just "#define unreachable() BUG()" and convert UD2 and
> similar to use __builtin_unreachable()?

Just remove unreachable() entirely at that point. But you're going to
have to update all the various arch code that does use it correctly :/

There are a few sites besides BUG() that need it; eg, long jumps when
bootstrapping etc.

But basically nothing outside of arch code should ever need or want
unreachable.