Re: [PATCH] drm/hyperv: Don't rely on screen_info.lfb_base for Gen1 VMs

From: Thomas Zimmermann
Date: Mon Sep 12 2022 - 03:04:02 EST

Hi

Am 11.09.22 um 18:21 schrieb Saurabh Singh Sengar:
On Sat, Sep 10, 2022 at 08:11:24PM +0200, Thomas Zimmermann wrote:
Hi

Am 09.09.22 um 16:43 schrieb Saurabh Sengar:
hyperv_setup_vram tries to remove conflicting framebuffer based on
'screen_info'. As observed in past due to some bug or wrong setting
in grub, the 'screen_info' fields may not be set for Gen1, and in such
cases drm_aperture_remove_conflicting_framebuffers will not do anything
useful.
For Gen1 VMs, it should always be possible to get framebuffer
conflict removed using PCI device instead.

Fixes: a0ab5abced55 ("drm/hyperv : Removing the restruction of VRAM allocation with PCI bar size")
Signed-off-by: Saurabh Sengar <ssengar@xxxxxxxxxxxxxxxxxxx>
---
drivers/gpu/drm/hyperv/hyperv_drm_drv.c | 24 ++++++++++++++++++++----
1 file changed, 20 insertions(+), 4 deletions(-)

diff --git a/drivers/gpu/drm/hyperv/hyperv_drm_drv.c b/drivers/gpu/drm/hyperv/hyperv_drm_drv.c
index 6d11e7938c83..b0cc974efa45 100644
--- a/drivers/gpu/drm/hyperv/hyperv_drm_drv.c
+++ b/drivers/gpu/drm/hyperv/hyperv_drm_drv.c
@@ -73,12 +73,28 @@ static int hyperv_setup_vram(struct hyperv_drm_device *hv,
struct hv_device *hdev)
{
struct drm_device *dev = &hv->dev;
+ struct pci_dev *pdev;
int ret;
- drm_aperture_remove_conflicting_framebuffers(screen_info.lfb_base,
- screen_info.lfb_size,
- false,
- &hyperv_driver);
+ if (efi_enabled(EFI_BOOT)) {
+ drm_aperture_remove_conflicting_framebuffers(screen_info.lfb_base,
+ screen_info.lfb_size,

Using screen_info here seems wrong in any case. You want to remove
the framebuffer devices that conflict with your driver, which might
be unrelated to screen_info. AFAICT the correct solution would
always retrieve the PCI device for removal (i.e., always do the else
branch).

In a Gen2 VM, the Hyper-V frame buffer device is presented only as a VMbus device.
It's not presented as a PCI device like it is in a Gen1 VM. This would have worked
if we had the frame buffer device available as PCI device in Gen2 but unfortunately
thats not the case here.

Thanks for explaining. There is an instance of struct hv_device passed to the probe function. I suspect you cannot get the framebuffer range from this instance (e.g., via the device's platform_data)?

If you absolutely can't get the actual memory region from the device, it's better to remove all framebuffers via drm_aperture_remove_framebuffers() than to use screen_info.

Best regards
Thomas


Regards,
Saurabh


Best regard
Thomas

+ false,
+ &hyperv_driver);
+ } else {
+ pdev = pci_get_device(PCI_VENDOR_ID_MICROSOFT, PCI_DEVICE_ID_HYPERV_VIDEO, NULL);
+ if (!pdev) {
+ drm_err(dev, "Unable to find PCI Hyper-V video\n");
+ return -ENODEV;
+ }
+
+ ret = drm_aperture_remove_conflicting_pci_framebuffers(pdev, &hyperv_driver);
+ pci_dev_put(pdev);
+ if (ret) {
+ drm_err(dev, "Not able to remove boot fb\n");
+ return ret;
+ }
+ }
hv->fb_size = (unsigned long)hv->mmio_megabytes * 1024 * 1024;

--
Thomas Zimmermann
Graphics Driver Developer
SUSE Software Solutions Germany GmbH
Maxfeldstr. 5, 90409 Nürnberg, Germany
(HRB 36809, AG Nürnberg)
Geschäftsführer: Ivo Totev




--
Thomas Zimmermann
Graphics Driver Developer
SUSE Software Solutions Germany GmbH
Maxfeldstr. 5, 90409 Nürnberg, Germany
(HRB 36809, AG Nürnberg)
Geschäftsführer: Ivo Totev

Attachment: OpenPGP_signature
Description: OpenPGP digital signature