1. Patchew
  2. linux
  3. View series

[PATCH] drm/sysfb: efidrm: Fix framebuffer mapping error handling

Chen Ni posted 1 patch 1 month ago 
drivers/gpu/drm/sysfb/efidrm.c | 42 ++++++++++++++++++++++++----------
1 file changed, 30 insertions(+), 12 deletions(-)
[PATCH] drm/sysfb: efidrm: Fix framebuffer mapping error handling
Posted by Chen Ni 1 month ago 
devm_memremap() returns an error pointer on failure, not NULL. Using
!screen_base to check for errors will miss error codes and treat invalid
pointers as valid addresses.

devm_memremap() returns system memory pointers (void *), while
devm_ioremap() returns I/O memory pointers (void __iomem *). The current
code incorrectly passes system memory pointers to
iosys_map_set_vaddr_iomem(), which expects I/O memory.

Fix these issues by:
- Properly checking IS_ERR() for devm_memremap() return values.
- Using iosys_map_set_vaddr_iomem() for I/O memory mappings (WC/UC).
- Using iosys_map_set_vaddr() for system memory mappings (WT/WB).

Fixes: 32ae90c66fb6 ("drm/sysfb: Add efidrm for EFI displays")
Signed-off-by: Chen Ni <nichen@iscas.ac.cn>
---
 drivers/gpu/drm/sysfb/efidrm.c | 42 ++++++++++++++++++++++++----------
 1 file changed, 30 insertions(+), 12 deletions(-)

diff --git a/drivers/gpu/drm/sysfb/efidrm.c b/drivers/gpu/drm/sysfb/efidrm.c
index 1114359a1e62..a1da841a8dd4 100644
--- a/drivers/gpu/drm/sysfb/efidrm.c
+++ b/drivers/gpu/drm/sysfb/efidrm.c
@@ -158,6 +158,7 @@ static struct efidrm_device *efidrm_device_create(struct drm_driver *drv,
 	struct drm_device *dev;
 	struct resource *mem = NULL;
 	void __iomem *screen_base = NULL;
+	void *sys_base = NULL;
 	struct drm_plane *primary_plane;
 	struct drm_crtc *crtc;
 	struct drm_encoder *encoder;
@@ -165,6 +166,7 @@ static struct efidrm_device *efidrm_device_create(struct drm_driver *drv,
 	unsigned long max_width, max_height;
 	size_t nformats;
 	int ret;
+	bool is_iomem = false;
 
 	dpy = dev_get_platdata(&pdev->dev);
 	if (!dpy)
@@ -244,21 +246,37 @@ static struct efidrm_device *efidrm_device_create(struct drm_driver *drv,
 
 	mem_flags = efidrm_get_mem_flags(dev, res->start, vsize);
 
-	if (mem_flags & EFI_MEMORY_WC)
+	if (mem_flags & EFI_MEMORY_WC) {
 		screen_base = devm_ioremap_wc(&pdev->dev, mem->start, resource_size(mem));
-	else if (mem_flags & EFI_MEMORY_UC)
+		is_iomem = true;
+		if (!screen_base)
+			return ERR_PTR(-ENOMEM);
+	} else if (mem_flags & EFI_MEMORY_UC) {
 		screen_base = devm_ioremap(&pdev->dev, mem->start, resource_size(mem));
-	else if (mem_flags & EFI_MEMORY_WT)
-		screen_base = devm_memremap(&pdev->dev, mem->start, resource_size(mem),
-					    MEMREMAP_WT);
-	else if (mem_flags & EFI_MEMORY_WB)
-		screen_base = devm_memremap(&pdev->dev, mem->start, resource_size(mem),
-					    MEMREMAP_WB);
-	else
+		is_iomem = true;
+		if (!screen_base)
+			return ERR_PTR(-ENOMEM);
+	} else if (mem_flags & EFI_MEMORY_WT) {
+		sys_base = devm_memremap(&pdev->dev, mem->start, resource_size(mem),
+					 MEMREMAP_WT);
+		is_iomem = false;
+		if (IS_ERR(sys_base))
+			return sys_base;
+	} else if (mem_flags & EFI_MEMORY_WB) {
+		sys_base = devm_memremap(&pdev->dev, mem->start, resource_size(mem),
+					 MEMREMAP_WB);
+		is_iomem = false;
+		if (IS_ERR(sys_base))
+			return sys_base;
+	} else {
 		drm_err(dev, "invalid mem_flags: 0x%llx\n", mem_flags);
-	if (!screen_base)
-		return ERR_PTR(-ENOMEM);
-	iosys_map_set_vaddr_iomem(&sysfb->fb_addr, screen_base);
+		return ERR_PTR(-EINVAL);
+	}
+
+	if (is_iomem)
+		iosys_map_set_vaddr_iomem(&sysfb->fb_addr, screen_base);
+	else
+		iosys_map_set_vaddr(&sysfb->fb_addr, sys_base);
 
 	/*
 	 * Modesetting
-- 
2.25.1
Re: [PATCH] drm/sysfb: efidrm: Fix framebuffer mapping error handling
Posted by Thomas Zimmermann 1 month ago 
Hi,

thanks for the patch.

Am 06.03.26 um 10:19 schrieb Chen Ni:
> devm_memremap() returns an error pointer on failure, not NULL. Using
> !screen_base to check for errors will miss error codes and treat invalid
> pointers as valid addresses.
>
> devm_memremap() returns system memory pointers (void *), while
> devm_ioremap() returns I/O memory pointers (void __iomem *). The current
> code incorrectly passes system memory pointers to
> iosys_map_set_vaddr_iomem(), which expects I/O memory.
>
> Fix these issues by:
> - Properly checking IS_ERR() for devm_memremap() return values.
> - Using iosys_map_set_vaddr_iomem() for I/O memory mappings (WC/UC).
> - Using iosys_map_set_vaddr() for system memory mappings (WT/WB).

That's quite a bit of changes. I don't like the is_iomem flag TBH. 
Rather handle each case in a single branch like that:

if (EFI_MEMORY_WC)
     void __iomem *screen_base = devm_ioremap_wc()

     if (!screen_base)
         return ERR_PTR(-ENXIO);
} else if (MEMORY_UC) {
    ...
} else if (MEMORY_WT)
     void *screen_base = devm_remap()

     if (IS_ERR(screen_base))
         return ERR_CAST(screen_base);
} else if (...) {
     ...
} else {
    drm_err(...);
    return -EINVAL;
}

This also gets the errno codes right.

>
> Fixes: 32ae90c66fb6 ("drm/sysfb: Add efidrm for EFI displays")
> Signed-off-by: Chen Ni <nichen@iscas.ac.cn>
> ---
>   drivers/gpu/drm/sysfb/efidrm.c | 42 ++++++++++++++++++++++++----------
>   1 file changed, 30 insertions(+), 12 deletions(-)
>
> diff --git a/drivers/gpu/drm/sysfb/efidrm.c b/drivers/gpu/drm/sysfb/efidrm.c
> index 1114359a1e62..a1da841a8dd4 100644
> --- a/drivers/gpu/drm/sysfb/efidrm.c
> +++ b/drivers/gpu/drm/sysfb/efidrm.c
> @@ -158,6 +158,7 @@ static struct efidrm_device *efidrm_device_create(struct drm_driver *drv,
>   	struct drm_device *dev;
>   	struct resource *mem = NULL;
>   	void __iomem *screen_base = NULL;
> +	void *sys_base = NULL;
>   	struct drm_plane *primary_plane;
>   	struct drm_crtc *crtc;
>   	struct drm_encoder *encoder;
> @@ -165,6 +166,7 @@ static struct efidrm_device *efidrm_device_create(struct drm_driver *drv,
>   	unsigned long max_width, max_height;
>   	size_t nformats;
>   	int ret;
> +	bool is_iomem = false;
>   
>   	dpy = dev_get_platdata(&pdev->dev);
>   	if (!dpy)
> @@ -244,21 +246,37 @@ static struct efidrm_device *efidrm_device_create(struct drm_driver *drv,
>   
>   	mem_flags = efidrm_get_mem_flags(dev, res->start, vsize);
>   
> -	if (mem_flags & EFI_MEMORY_WC)
> +	if (mem_flags & EFI_MEMORY_WC) {
>   		screen_base = devm_ioremap_wc(&pdev->dev, mem->start, resource_size(mem));
> -	else if (mem_flags & EFI_MEMORY_UC)
> +		is_iomem = true;
> +		if (!screen_base)
> +			return ERR_PTR(-ENOMEM);
> +	} else if (mem_flags & EFI_MEMORY_UC) {
>   		screen_base = devm_ioremap(&pdev->dev, mem->start, resource_size(mem));
> -	else if (mem_flags & EFI_MEMORY_WT)
> -		screen_base = devm_memremap(&pdev->dev, mem->start, resource_size(mem),
> -					    MEMREMAP_WT);
> -	else if (mem_flags & EFI_MEMORY_WB)
> -		screen_base = devm_memremap(&pdev->dev, mem->start, resource_size(mem),
> -					    MEMREMAP_WB);
> -	else
> +		is_iomem = true;
> +		if (!screen_base)
> +			return ERR_PTR(-ENOMEM);
> +	} else if (mem_flags & EFI_MEMORY_WT) {
> +		sys_base = devm_memremap(&pdev->dev, mem->start, resource_size(mem),
> +					 MEMREMAP_WT);
> +		is_iomem = false;
> +		if (IS_ERR(sys_base))
> +			return sys_base;
> +	} else if (mem_flags & EFI_MEMORY_WB) {
> +		sys_base = devm_memremap(&pdev->dev, mem->start, resource_size(mem),
> +					 MEMREMAP_WB);
> +		is_iomem = false;
> +		if (IS_ERR(sys_base))
> +			return sys_base;
> +	} else {
>   		drm_err(dev, "invalid mem_flags: 0x%llx\n", mem_flags);
> -	if (!screen_base)
> -		return ERR_PTR(-ENOMEM);
> -	iosys_map_set_vaddr_iomem(&sysfb->fb_addr, screen_base);
> +		return ERR_PTR(-EINVAL);

This errno code would only be correct in the case of the final else 
branch.  devm_memremap() use -ENXIO, so I think we should as well.

Best regards
Thomas

> +	}
> +
> +	if (is_iomem)
> +		iosys_map_set_vaddr_iomem(&sysfb->fb_addr, screen_base);
> +	else
> +		iosys_map_set_vaddr(&sysfb->fb_addr, sys_base);
>   
>   	/*
>   	 * Modesetting

-- 
--
Thomas Zimmermann
Graphics Driver Developer
SUSE Software Solutions Germany GmbH
Frankenstr. 146, 90461 Nürnberg, Germany, www.suse.com
GF: Jochen Jaser, Andrew McDonald, Werner Knoblich, (HRB 36809, AG Nürnberg)

Patchew 2.1-devel-ea86937

© 2016 - 2026 Red Hat, Inc.