[PATCH v3] video: screen_info: Relocate framebuffers behind PCI bridges

[PATCH v3] video: screen_info: Relocate framebuffers behind PCI bridges

[Thomas Zimmermann]

Apply PCI host-bridge window offsets to screen_info framebuffers. Fixes
invalid access to I/O memory.

Resources behind a PCI host bridge can be relocated by a certain offset
in the kernel's CPU address range used for I/O. The framebuffer memory
range stored in screen_info refers to the CPU addresses as seen during
boot (where the offset is 0). During boot up, firmware may assign a
different memory offset to the PCI host bridge and thereby relocating
the framebuffer address of the PCI graphics device as seen by the kernel.
The information in screen_info must be updated as well.

The helper pcibios_bus_to_resource() performs the relocation of the
screen_info's framebuffer resource (given in PCI bus addresses). The
result matches the I/O-memory resource of the PCI graphics device (given
in CPU addresses). As before, we store away the information necessary to
later update the information in screen_info itself.

Commit 78aa89d1dfba ("firmware/sysfb: Update screen_info for relocated
EFI framebuffers") added the code for updating screen_info. It is based
on similar functionality that pre-existed in efifb. Efifb uses a pointer
to the PCI resource, while the newer code does a memcpy of the region.
Hence efifb sees any updates to the PCI resource and avoids the issue.

v3:
- Only use struct pci_bus_region for PCI bus addresses (Bjorn)
- Clarify address semantics in commit messages and comments (Bjorn)
v2:
- Fixed tags (Takashi, Ivan)
- Updated information on efifb

Signed-off-by: Thomas Zimmermann <tzimmermann@suse.de>
Reviewed-by: Javier Martinez Canillas <javierm@redhat.com>
Reported-by: "Ivan T. Ivanov" <iivanov@suse.de>
Closes: https://bugzilla.suse.com/show_bug.cgi?id=1240696
Tested-by: "Ivan T. Ivanov" <iivanov@suse.de>
Fixes: 78aa89d1dfba ("firmware/sysfb: Update screen_info for relocated EFI framebuffers")
Cc: dri-devel@lists.freedesktop.org
Cc: <stable@vger.kernel.org> # v6.9+
---
 drivers/video/screen_info_pci.c | 79 +++++++++++++++++++++------------
 1 file changed, 50 insertions(+), 29 deletions(-)

diff --git a/drivers/video/screen_info_pci.c b/drivers/video/screen_info_pci.c
index 6c5833517141..66bfc1d0a6dc 100644
--- a/drivers/video/screen_info_pci.c
+++ b/drivers/video/screen_info_pci.c
@@ -7,8 +7,8 @@
 
 static struct pci_dev *screen_info_lfb_pdev;
 static size_t screen_info_lfb_bar;
-static resource_size_t screen_info_lfb_offset;
-static struct resource screen_info_lfb_res = DEFINE_RES_MEM(0, 0);
+static resource_size_t screen_info_lfb_res_start; // original start of resource
+static resource_size_t screen_info_lfb_offset; // framebuffer offset within resource
 
 static bool __screen_info_relocation_is_valid(const struct screen_info *si, struct resource *pr)
 {
@@ -31,7 +31,7 @@ void screen_info_apply_fixups(void)
 	if (screen_info_lfb_pdev) {
 		struct resource *pr = &screen_info_lfb_pdev->resource[screen_info_lfb_bar];
 
-		if (pr->start != screen_info_lfb_res.start) {
+		if (pr->start != screen_info_lfb_res_start) {
 			if (__screen_info_relocation_is_valid(si, pr)) {
 				/*
 				 * Only update base if we have an actual
@@ -47,46 +47,67 @@ void screen_info_apply_fixups(void)
 	}
 }
 
+static int __screen_info_lfb_pci_bus_region(const struct screen_info *si, unsigned int type,
+					    struct pci_bus_region *r)
+{
+	u64 base, size;
+
+	base = __screen_info_lfb_base(si);
+	if (!base)
+		return -EINVAL;
+
+	size = __screen_info_lfb_size(si, type);
+	if (!size)
+		return -EINVAL;
+
+	r->start = base;
+	r->end = base + size - 1;
+
+	return 0;
+}
+
 static void screen_info_fixup_lfb(struct pci_dev *pdev)
 {
 	unsigned int type;
-	struct resource res[SCREEN_INFO_MAX_RESOURCES];
-	size_t i, numres;
+	struct pci_bus_region bus_region;
 	int ret;
+	struct resource r = {
+		.flags = IORESOURCE_MEM,
+	};
+	const struct resource *pr;
 	const struct screen_info *si = &screen_info;
 
 	if (screen_info_lfb_pdev)
 		return; // already found
 
 	type = screen_info_video_type(si);
-	if (type != VIDEO_TYPE_EFI)
-		return; // only applies to EFI
+	if (!__screen_info_has_lfb(type))
+		return; // only applies to EFI; maybe VESA
 
-	ret = screen_info_resources(si, res, ARRAY_SIZE(res));
+	ret = __screen_info_lfb_pci_bus_region(si, type, &bus_region);
 	if (ret < 0)
 		return;
-	numres = ret;
 
-	for (i = 0; i < numres; ++i) {
-		struct resource *r = &res[i];
-		const struct resource *pr;
-
-		if (!(r->flags & IORESOURCE_MEM))
-			continue;
-		pr = pci_find_resource(pdev, r);
-		if (!pr)
-			continue;
-
-		/*
-		 * We've found a PCI device with the framebuffer
-		 * resource. Store away the parameters to track
-		 * relocation of the framebuffer aperture.
-		 */
-		screen_info_lfb_pdev = pdev;
-		screen_info_lfb_bar = pr - pdev->resource;
-		screen_info_lfb_offset = r->start - pr->start;
-		memcpy(&screen_info_lfb_res, r, sizeof(screen_info_lfb_res));
-	}
+	/*
+	 * Translate the PCI bus address to resource. Account
+	 * for an offset if the framebuffer is behind a PCI host
+	 * bridge.
+	 */
+	pcibios_bus_to_resource(pdev->bus, &r, &bus_region);
+
+	pr = pci_find_resource(pdev, &r);
+	if (!pr)
+		return;
+
+	/*
+	 * We've found a PCI device with the framebuffer
+	 * resource. Store away the parameters to track
+	 * relocation of the framebuffer aperture.
+	 */
+	screen_info_lfb_pdev = pdev;
+	screen_info_lfb_bar = pr - pdev->resource;
+	screen_info_lfb_offset = r.start - pr->start;
+	screen_info_lfb_res_start = bus_region.start;
 }
 DECLARE_PCI_FIXUP_CLASS_HEADER(PCI_ANY_ID, PCI_ANY_ID, PCI_BASE_CLASS_DISPLAY, 16,
 			       screen_info_fixup_lfb);
-- 
2.49.0

Re: [PATCH v3] video: screen_info: Relocate framebuffers behind PCI bridges

[Thomas Zimmermann]

If there are no further comment, I'll merge this patch via drm-misc-fixes.

Am 28.05.25 um 10:02 schrieb Thomas Zimmermann:
> Apply PCI host-bridge window offsets to screen_info framebuffers. Fixes
> invalid access to I/O memory.
>
> Resources behind a PCI host bridge can be relocated by a certain offset
> in the kernel's CPU address range used for I/O. The framebuffer memory
> range stored in screen_info refers to the CPU addresses as seen during
> boot (where the offset is 0). During boot up, firmware may assign a
> different memory offset to the PCI host bridge and thereby relocating
> the framebuffer address of the PCI graphics device as seen by the kernel.
> The information in screen_info must be updated as well.
>
> The helper pcibios_bus_to_resource() performs the relocation of the
> screen_info's framebuffer resource (given in PCI bus addresses). The
> result matches the I/O-memory resource of the PCI graphics device (given
> in CPU addresses). As before, we store away the information necessary to
> later update the information in screen_info itself.
>
> Commit 78aa89d1dfba ("firmware/sysfb: Update screen_info for relocated
> EFI framebuffers") added the code for updating screen_info. It is based
> on similar functionality that pre-existed in efifb. Efifb uses a pointer
> to the PCI resource, while the newer code does a memcpy of the region.
> Hence efifb sees any updates to the PCI resource and avoids the issue.
>
> v3:
> - Only use struct pci_bus_region for PCI bus addresses (Bjorn)
> - Clarify address semantics in commit messages and comments (Bjorn)
> v2:
> - Fixed tags (Takashi, Ivan)
> - Updated information on efifb
>
> Signed-off-by: Thomas Zimmermann <tzimmermann@suse.de>
> Reviewed-by: Javier Martinez Canillas <javierm@redhat.com>
> Reported-by: "Ivan T. Ivanov" <iivanov@suse.de>
> Closes: https://bugzilla.suse.com/show_bug.cgi?id=1240696
> Tested-by: "Ivan T. Ivanov" <iivanov@suse.de>
> Fixes: 78aa89d1dfba ("firmware/sysfb: Update screen_info for relocated EFI framebuffers")
> Cc: dri-devel@lists.freedesktop.org
> Cc: <stable@vger.kernel.org> # v6.9+
> ---
>   drivers/video/screen_info_pci.c | 79 +++++++++++++++++++++------------
>   1 file changed, 50 insertions(+), 29 deletions(-)
>
> diff --git a/drivers/video/screen_info_pci.c b/drivers/video/screen_info_pci.c
> index 6c5833517141..66bfc1d0a6dc 100644
> --- a/drivers/video/screen_info_pci.c
> +++ b/drivers/video/screen_info_pci.c
> @@ -7,8 +7,8 @@
>   
>   static struct pci_dev *screen_info_lfb_pdev;
>   static size_t screen_info_lfb_bar;
> -static resource_size_t screen_info_lfb_offset;
> -static struct resource screen_info_lfb_res = DEFINE_RES_MEM(0, 0);
> +static resource_size_t screen_info_lfb_res_start; // original start of resource
> +static resource_size_t screen_info_lfb_offset; // framebuffer offset within resource
>   
>   static bool __screen_info_relocation_is_valid(const struct screen_info *si, struct resource *pr)
>   {
> @@ -31,7 +31,7 @@ void screen_info_apply_fixups(void)
>   	if (screen_info_lfb_pdev) {
>   		struct resource *pr = &screen_info_lfb_pdev->resource[screen_info_lfb_bar];
>   
> -		if (pr->start != screen_info_lfb_res.start) {
> +		if (pr->start != screen_info_lfb_res_start) {
>   			if (__screen_info_relocation_is_valid(si, pr)) {
>   				/*
>   				 * Only update base if we have an actual
> @@ -47,46 +47,67 @@ void screen_info_apply_fixups(void)
>   	}
>   }
>   
> +static int __screen_info_lfb_pci_bus_region(const struct screen_info *si, unsigned int type,
> +					    struct pci_bus_region *r)
> +{
> +	u64 base, size;
> +
> +	base = __screen_info_lfb_base(si);
> +	if (!base)
> +		return -EINVAL;
> +
> +	size = __screen_info_lfb_size(si, type);
> +	if (!size)
> +		return -EINVAL;
> +
> +	r->start = base;
> +	r->end = base + size - 1;
> +
> +	return 0;
> +}
> +
>   static void screen_info_fixup_lfb(struct pci_dev *pdev)
>   {
>   	unsigned int type;
> -	struct resource res[SCREEN_INFO_MAX_RESOURCES];
> -	size_t i, numres;
> +	struct pci_bus_region bus_region;
>   	int ret;
> +	struct resource r = {
> +		.flags = IORESOURCE_MEM,
> +	};
> +	const struct resource *pr;
>   	const struct screen_info *si = &screen_info;
>   
>   	if (screen_info_lfb_pdev)
>   		return; // already found
>   
>   	type = screen_info_video_type(si);
> -	if (type != VIDEO_TYPE_EFI)
> -		return; // only applies to EFI
> +	if (!__screen_info_has_lfb(type))
> +		return; // only applies to EFI; maybe VESA
>   
> -	ret = screen_info_resources(si, res, ARRAY_SIZE(res));
> +	ret = __screen_info_lfb_pci_bus_region(si, type, &bus_region);
>   	if (ret < 0)
>   		return;
> -	numres = ret;
>   
> -	for (i = 0; i < numres; ++i) {
> -		struct resource *r = &res[i];
> -		const struct resource *pr;
> -
> -		if (!(r->flags & IORESOURCE_MEM))
> -			continue;
> -		pr = pci_find_resource(pdev, r);
> -		if (!pr)
> -			continue;
> -
> -		/*
> -		 * We've found a PCI device with the framebuffer
> -		 * resource. Store away the parameters to track
> -		 * relocation of the framebuffer aperture.
> -		 */
> -		screen_info_lfb_pdev = pdev;
> -		screen_info_lfb_bar = pr - pdev->resource;
> -		screen_info_lfb_offset = r->start - pr->start;
> -		memcpy(&screen_info_lfb_res, r, sizeof(screen_info_lfb_res));
> -	}
> +	/*
> +	 * Translate the PCI bus address to resource. Account
> +	 * for an offset if the framebuffer is behind a PCI host
> +	 * bridge.
> +	 */
> +	pcibios_bus_to_resource(pdev->bus, &r, &bus_region);
> +
> +	pr = pci_find_resource(pdev, &r);
> +	if (!pr)
> +		return;
> +
> +	/*
> +	 * We've found a PCI device with the framebuffer
> +	 * resource. Store away the parameters to track
> +	 * relocation of the framebuffer aperture.
> +	 */
> +	screen_info_lfb_pdev = pdev;
> +	screen_info_lfb_bar = pr - pdev->resource;
> +	screen_info_lfb_offset = r.start - pr->start;
> +	screen_info_lfb_res_start = bus_region.start;
>   }
>   DECLARE_PCI_FIXUP_CLASS_HEADER(PCI_ANY_ID, PCI_ANY_ID, PCI_BASE_CLASS_DISPLAY, 16,
>   			       screen_info_fixup_lfb);

-- 
--
Thomas Zimmermann
Graphics Driver Developer
SUSE Software Solutions Germany GmbH
Frankenstrasse 146, 90461 Nuernberg, Germany
GF: Ivo Totev, Andrew Myers, Andrew McDonald, Boudien Moerman
HRB 36809 (AG Nuernberg)