[PATCH v2] io-mapping: Indicate mapping failure

[Andrew Morton]

On Tue, 21 Jul 2020 13:19:36 -0400 "Michael J. Ruhl" <michael.j.ruhl at intel.com> wrote:

> The !ATOMIC_IOMAP version of io_maping_init_wc will always return
> success, even when the ioremap fails.
> 
> Since the ATOMIC_IOMAP version returns NULL when the init fails, and
> callers check for a NULL return on error this is unexpected.
> 
> During a device probe, where the ioremap failed, a crash can look
> like this:
> 
> BUG: unable to handle page fault for address: 0000000000210000
>  #PF: supervisor write access in kernel mode
>  #PF: error_code(0x0002) - not-present page
>  Oops: 0002 [#1] PREEMPT SMP
>  CPU: 0 PID: 177 Comm:
>  RIP: 0010:fill_page_dma [i915]
>   gen8_ppgtt_create [i915]
>   i915_ppgtt_create [i915]
>   intel_gt_init [i915]
>   i915_gem_init [i915]
>   i915_driver_probe [i915]
>   pci_device_probe
>   really_probe
>   driver_probe_device
> 
> The remap failure occurred much earlier in the probe.  If it had
> been propagated, the driver would have exited with an error.
> 
> Return NULL on ioremap failure.
>
> ...
>
> --- a/include/linux/io-mapping.h
> +++ b/include/linux/io-mapping.h
> @@ -118,7 +118,7 @@ io_mapping_init_wc(struct io_mapping *iomap,
>  	iomap->prot = pgprot_noncached(PAGE_KERNEL);
>  #endif
>  
> -	return iomap;
> +	return iomap->iomem ? iomap : NULL;
>  }
>  
>  static inline void

LGTM.  However I do think it would be stylistically better/typical to
detect and handle the error early, rather than to blunder on,
pointlessly initializing things?

--- a/include/linux/io-mapping.h~io-mapping-indicate-mapping-failure-fix
+++ a/include/linux/io-mapping.h
@@ -107,9 +107,12 @@ io_mapping_init_wc(struct io_mapping *io
 		   resource_size_t base,
 		   unsigned long size)
 {
+	iomap->iomem = ioremap_wc(base, size);
+	if (!iomap->iomem)
+		return NULL;
+
 	iomap->base = base;
 	iomap->size = size;
-	iomap->iomem = ioremap_wc(base, size);
 #if defined(pgprot_noncached_wc) /* archs can't agree on a name ... */
 	iomap->prot = pgprot_noncached_wc(PAGE_KERNEL);
 #elif defined(pgprot_writecombine)
@@ -118,7 +121,7 @@ io_mapping_init_wc(struct io_mapping *io
 	iomap->prot = pgprot_noncached(PAGE_KERNEL);
 #endif
 
-	return iomap->iomem ? iomap : NULL;
+	return iomap;
 }
 
 static inline void
_