[PATCH 0/4] hyperv, sysfb: Do not use screen_info in drivers

[PATCH 0/4] hyperv, sysfb: Do not use screen_info in drivers

[Thomas Zimmermann]

The global screen_info state is only meant for architecture and
firmware code. Replace its use in hyperv graphics drivers with the
correct aperture helpers.

Patches 1 and 2 update hyperv-drm and hyperv-fb to use the correct
aperture helpers instead of screen_info for removing existing firmware
framebuffers.

Hyperv-fb also modifies screen_info for better use with kexec. While
that update makes sense, it's not supposed to be done by the driver.
Patch 3 adds similar code to sysfb and patch 4 removes the code from
the driver.

An intented side effect of this patchset is that all systems now
benefit from better kexec support. After rebooting with kexec, the
kernel operated on stale settings on screen_info. Patch 3 fixes this
and the kexec kernel will use screen_info in any meaningful way.

Thomas Zimmermann (4):
  drm/hyperv: Remove firmware framebuffers with aperture helper
  fbdev/hyperv_fb: Remove firmware framebuffers with aperture helpers
  firmware/sysfb: Clear screen_info state after consuming it
  fbdev/hyperv_fb: Do not clear global screen_info

 drivers/firmware/sysfb.c                | 14 +++++++++++++-
 drivers/gpu/drm/hyperv/hyperv_drm_drv.c |  8 ++------
 drivers/video/fbdev/hyperv_fb.c         | 20 +++++++-------------
 3 files changed, 22 insertions(+), 20 deletions(-)


base-commit: 25232eb8a9ac7fa0dac7e846a4bf7fba2b6db39a
-- 
2.43.0

[PATCH 1/4] drm/hyperv: Remove firmware framebuffers with aperture helper

[Thomas Zimmermann]

Replace use of screen_info state with the correct interface from
the aperture helpers. The state is only for architecture and firmware
code. It is not guaranteed to contain valid data. Drivers are thus
not allowed to use it.

For removing conflicting firmware framebuffers, there are aperture
helpers. Hence replace screen_info with the correct function that will
remove conflicting framebuffers for the hyperv-drm driver. Also
move the call to the correct place within the driver.

Signed-off-by: Thomas Zimmermann <tzimmermann@suse.de>
---
 drivers/gpu/drm/hyperv/hyperv_drm_drv.c | 8 ++------
 1 file changed, 2 insertions(+), 6 deletions(-)

diff --git a/drivers/gpu/drm/hyperv/hyperv_drm_drv.c b/drivers/gpu/drm/hyperv/hyperv_drm_drv.c
index d511d17c5bdf..cff85086f2d6 100644
--- a/drivers/gpu/drm/hyperv/hyperv_drm_drv.c
+++ b/drivers/gpu/drm/hyperv/hyperv_drm_drv.c
@@ -7,7 +7,6 @@
 #include <linux/hyperv.h>
 #include <linux/module.h>
 #include <linux/pci.h>
-#include <linux/screen_info.h>
 
 #include <drm/drm_aperture.h>
 #include <drm/drm_atomic_helper.h>
@@ -73,11 +72,6 @@ static int hyperv_setup_vram(struct hyperv_drm_device *hv,
 	struct drm_device *dev = &hv->dev;
 	int ret;
 
-	if (IS_ENABLED(CONFIG_SYSFB))
-		drm_aperture_remove_conflicting_framebuffers(screen_info.lfb_base,
-							     screen_info.lfb_size,
-							     &hyperv_driver);
-
 	hv->fb_size = (unsigned long)hv->mmio_megabytes * 1024 * 1024;
 
 	ret = vmbus_allocate_mmio(&hv->mem, hdev, 0, -1, hv->fb_size, 0x100000,
@@ -130,6 +124,8 @@ static int hyperv_vmbus_probe(struct hv_device *hdev,
 		goto err_hv_set_drv_data;
 	}
 
+	drm_aperture_remove_framebuffers(&hyperv_driver);
+
 	ret = hyperv_setup_vram(hv, hdev);
 	if (ret)
 		goto err_vmbus_close;
-- 
2.43.0

[PATCH 2/4] fbdev/hyperv_fb: Remove firmware framebuffers with aperture helpers

[Thomas Zimmermann]

Replace use of screen_info state with the correct interfaces from
the aperture helpers. The state is only for architecture and firmware
code. It is not guaranteed to contain valid data. Drivers are thus
not allowed to use it.

For removing conflicting firmware framebuffers, there are aperture
helpers. Hence replace screen_info with the correct functions that will
remove conflicting framebuffers for the hypervfb driver. For GEN1 PCI
devices, the driver reads the framebuffer base and size from the PCI
BAR, and uses the range for removing the firmware framebuffer. For
GEN2 VMBUS devices no range can be detected, so the driver clears all
firmware framebuffers.

Signed-off-by: Thomas Zimmermann <tzimmermann@suse.de>
---
 drivers/video/fbdev/hyperv_fb.c | 11 ++++++-----
 1 file changed, 6 insertions(+), 5 deletions(-)

diff --git a/drivers/video/fbdev/hyperv_fb.c b/drivers/video/fbdev/hyperv_fb.c
index a80939fe2ee6..76c956b9a321 100644
--- a/drivers/video/fbdev/hyperv_fb.c
+++ b/drivers/video/fbdev/hyperv_fb.c
@@ -975,7 +975,8 @@ static int hvfb_getmem(struct hv_device *hdev, struct fb_info *info)
 	struct pci_dev *pdev  = NULL;
 	void __iomem *fb_virt;
 	int gen2vm = efi_enabled(EFI_BOOT);
-	resource_size_t base, size;
+	resource_size_t base = 0;
+	resource_size_t size = 0;
 	phys_addr_t paddr;
 	int ret;
 
@@ -1010,9 +1011,6 @@ static int hvfb_getmem(struct hv_device *hdev, struct fb_info *info)
 			goto getmem_done;
 		}
 		pr_info("Unable to allocate enough contiguous physical memory on Gen 1 VM. Using MMIO instead.\n");
-	} else if (IS_ENABLED(CONFIG_SYSFB)) {
-		base = screen_info.lfb_base;
-		size = screen_info.lfb_size;
 	} else {
 		goto err1;
 	}
@@ -1056,7 +1054,10 @@ static int hvfb_getmem(struct hv_device *hdev, struct fb_info *info)
 	info->screen_size = dio_fb_size;
 
 getmem_done:
-	aperture_remove_conflicting_devices(base, size, KBUILD_MODNAME);
+	if (base && size)
+		aperture_remove_conflicting_devices(base, size, KBUILD_MODNAME);
+	else
+		aperture_remove_all_conflicting_devices(KBUILD_MODNAME);
 
 	if (!gen2vm) {
 		pci_dev_put(pdev);
-- 
2.43.0

[PATCH 3/4] firmware/sysfb: Clear screen_info state after consuming it

[Thomas Zimmermann]

After consuming the global screen_info_state in sysfb_init(), the
created platform device maintains the firmware framebuffer. Clear
screen_info to avoid conflicting access. Subsequent kexec reboots
now ignore the firmware framebuffer.

Signed-off-by: Thomas Zimmermann <tzimmermann@suse.de>
---
 drivers/firmware/sysfb.c | 14 +++++++++++++-
 1 file changed, 13 insertions(+), 1 deletion(-)

diff --git a/drivers/firmware/sysfb.c b/drivers/firmware/sysfb.c
index 82fcfd29bc4d..19706bd2642a 100644
--- a/drivers/firmware/sysfb.c
+++ b/drivers/firmware/sysfb.c
@@ -71,7 +71,7 @@ EXPORT_SYMBOL_GPL(sysfb_disable);
 
 static __init int sysfb_init(void)
 {
-	struct screen_info *si = &screen_info;
+	const struct screen_info *si = &screen_info;
 	struct simplefb_platform_data mode;
 	const char *name;
 	bool compatible;
@@ -119,6 +119,18 @@ static __init int sysfb_init(void)
 	if (ret)
 		goto err;
 
+	/*
+	 * The firmware framebuffer is now maintained by the created
+	 * device. Disable screen_info after we've consumed it. Prevents
+	 * invalid access during kexec reboots.
+	 *
+	 * TODO: Vgacon still relies on the global screen_info. Make
+	 *       vgacon work with the platform device, so we can clear
+	 *       the screen_info unconditionally.
+	 */
+	if (strcmp(name, "platform-framebuffer"))
+		screen_info.orig_video_isVGA = 0;
+
 	goto unlock_mutex;
 err:
 	platform_device_put(pd);
-- 
2.43.0

[PATCH 4/4] fbdev/hyperv_fb: Do not clear global screen_info

[Thomas Zimmermann]

Do not clear the global instance of screen_info. If necessary, clearing
fields in screen_info should be done by architecture or firmware code
that maintains the firmware framebuffer.

Signed-off-by: Thomas Zimmermann <tzimmermann@suse.de>
---
 drivers/video/fbdev/hyperv_fb.c | 9 +--------
 1 file changed, 1 insertion(+), 8 deletions(-)

diff --git a/drivers/video/fbdev/hyperv_fb.c b/drivers/video/fbdev/hyperv_fb.c
index 76c956b9a321..7d5717805c0b 100644
--- a/drivers/video/fbdev/hyperv_fb.c
+++ b/drivers/video/fbdev/hyperv_fb.c
@@ -48,7 +48,6 @@
 #include <linux/aperture.h>
 #include <linux/module.h>
 #include <linux/kernel.h>
-#include <linux/screen_info.h>
 #include <linux/vmalloc.h>
 #include <linux/init.h>
 #include <linux/completion.h>
@@ -1059,14 +1058,8 @@ static int hvfb_getmem(struct hv_device *hdev, struct fb_info *info)
 	else
 		aperture_remove_all_conflicting_devices(KBUILD_MODNAME);
 
-	if (!gen2vm) {
+	if (!gen2vm)
 		pci_dev_put(pdev);
-	} else if (IS_ENABLED(CONFIG_SYSFB)) {
-		/* framebuffer is reallocated, clear screen_info to avoid misuse from kexec */
-		screen_info.lfb_size = 0;
-		screen_info.lfb_base = 0;
-		screen_info.orig_video_isVGA = 0;
-	}
 
 	return 0;
 
-- 
2.43.0

Re: [PATCH 1/4] drm/hyperv: Remove firmware framebuffers with aperture helper

[Javier Martinez Canillas]

Thomas Zimmermann <tzimmermann@suse.de> writes:

Hello Thomas,

> Replace use of screen_info state with the correct interface from
> the aperture helpers. The state is only for architecture and firmware
> code. It is not guaranteed to contain valid data. Drivers are thus
> not allowed to use it.
>
> For removing conflicting firmware framebuffers, there are aperture
> helpers. Hence replace screen_info with the correct function that will
> remove conflicting framebuffers for the hyperv-drm driver. Also
> move the call to the correct place within the driver.
>
> Signed-off-by: Thomas Zimmermann <tzimmermann@suse.de>
> ---

Reviewed-by: Javier Martinez Canillas <javierm@redhat.com>

-- 
Best regards,

Javier Martinez Canillas
Core Platforms
Red Hat

Re: [PATCH 2/4] fbdev/hyperv_fb: Remove firmware framebuffers with aperture helpers

[Javier Martinez Canillas]

Thomas Zimmermann <tzimmermann@suse.de> writes:

> Replace use of screen_info state with the correct interfaces from
> the aperture helpers. The state is only for architecture and firmware
> code. It is not guaranteed to contain valid data. Drivers are thus
> not allowed to use it.
>
> For removing conflicting firmware framebuffers, there are aperture
> helpers. Hence replace screen_info with the correct functions that will
> remove conflicting framebuffers for the hypervfb driver. For GEN1 PCI
> devices, the driver reads the framebuffer base and size from the PCI
> BAR, and uses the range for removing the firmware framebuffer. For
> GEN2 VMBUS devices no range can be detected, so the driver clears all
> firmware framebuffers.
>
> Signed-off-by: Thomas Zimmermann <tzimmermann@suse.de>
> ---

Reviewed-by: Javier Martinez Canillas <javierm@redhat.com>

-- 
Best regards,

Javier Martinez Canillas
Core Platforms
Red Hat

Re: [PATCH 1/4] drm/hyperv: Remove firmware framebuffers with aperture helper

[Helge Deller]

On 1/8/24 10:26, Javier Martinez Canillas wrote:
> Thomas Zimmermann <tzimmermann@suse.de> writes:
>
> Hello Thomas,
>
>> Replace use of screen_info state with the correct interface from
>> the aperture helpers. The state is only for architecture and firmware
>> code. It is not guaranteed to contain valid data. Drivers are thus
>> not allowed to use it.
>>
>> For removing conflicting firmware framebuffers, there are aperture
>> helpers. Hence replace screen_info with the correct function that will
>> remove conflicting framebuffers for the hyperv-drm driver. Also
>> move the call to the correct place within the driver.
>>
>> Signed-off-by: Thomas Zimmermann <tzimmermann@suse.de>
>> ---
>
> Reviewed-by: Javier Martinez Canillas <javierm@redhat.com>

Series applied to fbdev git tree.

Thanks,
Helge

Re: [PATCH 3/4] firmware/sysfb: Clear screen_info state after consuming it

[Javier Martinez Canillas]

Thomas Zimmermann <tzimmermann@suse.de> writes:

> After consuming the global screen_info_state in sysfb_init(), the
> created platform device maintains the firmware framebuffer. Clear
> screen_info to avoid conflicting access. Subsequent kexec reboots
> now ignore the firmware framebuffer.
>
> Signed-off-by: Thomas Zimmermann <tzimmermann@suse.de>
> ---

Reviewed-by: Javier Martinez Canillas <javierm@redhat.com>

-- 
Best regards,

Javier Martinez Canillas
Core Platforms
Red Hat

Re: [PATCH 4/4] fbdev/hyperv_fb: Do not clear global screen_info

[Javier Martinez Canillas]

Thomas Zimmermann <tzimmermann@suse.de> writes:

> Do not clear the global instance of screen_info. If necessary, clearing
> fields in screen_info should be done by architecture or firmware code
> that maintains the firmware framebuffer.
>
> Signed-off-by: Thomas Zimmermann <tzimmermann@suse.de>
> ---

Reviewed-by: Javier Martinez Canillas <javierm@redhat.com>

-- 
Best regards,

Javier Martinez Canillas
Core Platforms
Red Hat