| Message ID | 20220818142508.402273-1-vkuznets@redhat.com (mailing list archive) |
|---|---|
| Headers | show |
| Series | Drivers: hv: Avoid allocating MMIO from framebuffer region for other passed through PCI devices | expand |
From: Vitaly Kuznetsov <vkuznets@redhat.com> Sent: Thursday, August 18, 2022 7:25 AM > > When drm_aperture_remove_conflicting_pci_framebuffers() fails, 'pdev' > needs to be released with pci_dev_put(). > > Fixes: 76c56a5affeb ("drm/hyperv: Add DRM driver for hyperv synthetic video device") > Signed-off-by: Vitaly Kuznetsov <vkuznets@redhat.com> > --- > drivers/gpu/drm/hyperv/hyperv_drm_drv.c | 2 +- > 1 file changed, 1 insertion(+), 1 deletion(-) > > diff --git a/drivers/gpu/drm/hyperv/hyperv_drm_drv.c > b/drivers/gpu/drm/hyperv/hyperv_drm_drv.c > index 46f6c454b820..ca4e517b95ca 100644 > --- a/drivers/gpu/drm/hyperv/hyperv_drm_drv.c > +++ b/drivers/gpu/drm/hyperv/hyperv_drm_drv.c > @@ -82,7 +82,7 @@ static int hyperv_setup_gen1(struct hyperv_drm_device *hv) > ret = drm_aperture_remove_conflicting_pci_framebuffers(pdev, > &hyperv_driver); > if (ret) { > drm_err(dev, "Not able to remove boot fb\n"); > - return ret; > + goto error; > } > > if (pci_request_region(pdev, 0, DRIVER_NAME) != 0) > -- > 2.37.1 This patch appears to be obsoleted by commit a0ab5abced55 that was merged into 6.0-rc1. Of course, it does beg the question of why the original function hyperv_setup_gen2(), which is now renamed to hyperv_setup_vram(), doesn't check the return value from drm_aperture_remove_conflicting_framebuffers(). Michael
"Michael Kelley (LINUX)" <mikelley@microsoft.com> writes: > From: Vitaly Kuznetsov <vkuznets@redhat.com> Sent: Thursday, August 18, 2022 7:25 AM >> >> When drm_aperture_remove_conflicting_pci_framebuffers() fails, 'pdev' >> needs to be released with pci_dev_put(). >> >> Fixes: 76c56a5affeb ("drm/hyperv: Add DRM driver for hyperv synthetic video device") >> Signed-off-by: Vitaly Kuznetsov <vkuznets@redhat.com> >> --- >> drivers/gpu/drm/hyperv/hyperv_drm_drv.c | 2 +- >> 1 file changed, 1 insertion(+), 1 deletion(-) >> >> diff --git a/drivers/gpu/drm/hyperv/hyperv_drm_drv.c >> b/drivers/gpu/drm/hyperv/hyperv_drm_drv.c >> index 46f6c454b820..ca4e517b95ca 100644 >> --- a/drivers/gpu/drm/hyperv/hyperv_drm_drv.c >> +++ b/drivers/gpu/drm/hyperv/hyperv_drm_drv.c >> @@ -82,7 +82,7 @@ static int hyperv_setup_gen1(struct hyperv_drm_device *hv) >> ret = drm_aperture_remove_conflicting_pci_framebuffers(pdev, >> &hyperv_driver); >> if (ret) { >> drm_err(dev, "Not able to remove boot fb\n"); >> - return ret; >> + goto error; >> } >> >> if (pci_request_region(pdev, 0, DRIVER_NAME) != 0) >> -- >> 2.37.1 > > This patch appears to be obsoleted by commit a0ab5abced55 > that was merged into 6.0-rc1. Of course, it does beg the question of > why the original function hyperv_setup_gen2(), which is now renamed > to hyperv_setup_vram(), doesn't check the return value from > drm_aperture_remove_conflicting_framebuffers(). AFAICT this commit (which I've obviously missed) also solves the worst issue I'm trying to address with this series: conflict between framebuffer and SR-IOV VF config space. It would probably still make sense to reserve the whole FB region on Gen1 first thing and use it as-is for DRM/FB later (Patches 3-4).
Passed through PCI device sometimes misbehave on Gen1 VMs when Hyper-V DRM driver is also loaded. Looking at IOMEM assignment, we can see e.g. $ cat /proc/iomem ... f8000000-fffbffff : PCI Bus 0000:00 f8000000-fbffffff : 0000:00:08.0 f8000000-f8001fff : bb8c4f33-2ba2-4808-9f7f-02f3b4da22fe ... fe0000000-fffffffff : PCI Bus 0000:00 fe0000000-fe07fffff : bb8c4f33-2ba2-4808-9f7f-02f3b4da22fe fe0000000-fe07fffff : 2ba2:00:02.0 fe0000000-fe07fffff : mlx4_core the interesting part is the 'f8000000' region as it is actually the VM's framebuffer: $ lspci -v ... 0000:00:08.0 VGA compatible controller: Microsoft Corporation Hyper-V virtual VGA (prog-if 00 [VGA controller]) Flags: bus master, fast devsel, latency 0, IRQ 11 Memory at f8000000 (32-bit, non-prefetchable) [size=64M] ... hv_vmbus: registering driver hyperv_drm hyperv_drm 5620e0c7-8062-4dce-aeb7-520c7ef76171: [drm] Synthvid Version major 3, minor 5 hyperv_drm 0000:00:08.0: vgaarb: deactivate vga console hyperv_drm 0000:00:08.0: BAR 0: can't reserve [mem 0xf8000000-0xfbffffff] hyperv_drm 5620e0c7-8062-4dce-aeb7-520c7ef76171: [drm] Cannot request framebuffer, boot fb still active? Note: "Cannot request framebuffer" is not a fatal error in hyperv_setup_gen1() as the code assumes there's some other framebuffer device there but we actually have some other PCI device (mlx4 in this case) config space there! Resolve the issue by always reserving FB region on Gen1 VMs (PATCH3) and making sure we never allocate anything besides framebuffer from there (PATCH4). PATCH1 is a preparatory change, PATCH2 fixes a loosely related issue in Hyper-V DRM driver. Vitaly Kuznetsov (4): Drivers: hv: Move legacy Hyper-V PCI video device's ids to linux/hyperv.h drm/hyperv: Don't forget to put PCI device when removing conflicting FB fails Drivers: hv: Always reserve framebuffer region for Gen1 VMs Drivers: hv: Never allocate anything besides framebuffer from framebuffer memory region drivers/gpu/drm/hyperv/hyperv_drm_drv.c | 5 +-- drivers/hv/vmbus_drv.c | 57 ++++++++++++++++++------- drivers/video/fbdev/hyperv_fb.c | 4 -- include/linux/hyperv.h | 4 ++ 4 files changed, 47 insertions(+), 23 deletions(-)