From: Vitaly Kuznetsov <vkuznets@xxxxxxxxxx> Sent: Thursday, August 18, 2022 7:25 AM > > vmbus_reserve_fb() tries reserving framebuffer region iff > screen_info.lfb_base is set. Gen2 VMs seem to have it set by EFI fb > but on Gen1 VM it is observed to be zero. FWIW, in a Gen1 VM, whether screen_info.lfb_base is set depends on what grub sets up, which in turn seems to depend on the gfxpayload= setting in grub.cfg and certain versions of grub. There are cases where it is observed to be zero, but from our experiments it's not all cases. In a Gen2 VM, there's an edge case where the frame buffer has been moved, and a kexec() kernel may see the moved location instead of what was set by EFI. See https://lore.kernel.org/all/20201014092429.1415040-1-kasong@xxxxxxxxxx/ I think these points may be worth recording in the commit message here so that there's accurate record for the future. The Hyper-V and grub idiosyncrasies make this a very tricky area. > In fact, we do not need to > rely on some other video driver setting it correctly as Gen1 VMs have > a dedicated PCI device to look at. Both Hyper-V DRM and Hyper-V FB > drivers get framebuffer base from this PCI device already so Vmbus > driver can do the same trick. > > Check for legacy PCI video device presence and reserve the whole > region for framebuffer. > > Signed-off-by: Vitaly Kuznetsov <vkuznets@xxxxxxxxxx> > --- > drivers/hv/vmbus_drv.c | 47 +++++++++++++++++++++++++++++------------- > 1 file changed, 33 insertions(+), 14 deletions(-) > > diff --git a/drivers/hv/vmbus_drv.c b/drivers/hv/vmbus_drv.c > index 547ae334e5cd..6edaeefa2c3c 100644 > --- a/drivers/hv/vmbus_drv.c > +++ b/drivers/hv/vmbus_drv.c > @@ -35,6 +35,7 @@ > #include <linux/kernel.h> > #include <linux/syscore_ops.h> > #include <linux/dma-map-ops.h> > +#include <linux/pci.h> > #include <clocksource/hyperv_timer.h> > #include "hyperv_vmbus.h" > > @@ -2258,26 +2259,44 @@ static int vmbus_acpi_remove(struct acpi_device *device) > > static void vmbus_reserve_fb(void) > { > - int size; > + resource_size_t start = 0, size; > + struct pci_dev *pdev; > + > + if (efi_enabled(EFI_BOOT)) { > + /* Gen2 VM: get FB base from EFI framebuffer */ > + start = screen_info.lfb_base; > + size = max_t(__u32, screen_info.lfb_size, 0x800000); > + } else { > + /* Gen1 VM: get FB base from PCI */ > + pdev = pci_get_device(PCI_VENDOR_ID_MICROSOFT, > + PCI_DEVICE_ID_HYPERV_VIDEO, NULL); > + if (!pdev) > + return; > + > + if (!(pdev->resource[0].flags & IORESOURCE_MEM)) > + return; Doesn't this error exit need a pci_dev_put(pdev)? Or maybe reverse the test like this, and the later check for !start will do the error exit. if (pdev->resource[0].flags & IORESOURCE_MEM) { start = pci_resource_start(pdev, 0); size = pci_resource_len(pdev, 0); } > + > + start = pci_resource_start(pdev, 0); > + size = pci_resource_len(pdev, 0); > + > + /* > + * Release the PCI device so hyperv_drm or hyperv_fb driver can > + * grab it later. > + */ > + pci_dev_put(pdev); > + } > + > + if (!start) > + return; > + > /* > * Make a claim for the frame buffer in the resource tree under the > * first node, which will be the one below 4GB. The length seems to > * be underreported, particularly in a Generation 1 VM. So start out > * reserving a larger area and make it smaller until it succeeds. > */ > - > - if (screen_info.lfb_base) { > - if (efi_enabled(EFI_BOOT)) > - size = max_t(__u32, screen_info.lfb_size, 0x800000); > - else > - size = max_t(__u32, screen_info.lfb_size, 0x4000000); > - > - for (; !fb_mmio && (size >= 0x100000); size >>= 1) { > - fb_mmio = __request_region(hyperv_mmio, > - screen_info.lfb_base, size, > - fb_mmio_name, 0); > - } > - } > + for (; !fb_mmio && (size >= 0x100000); size >>= 1) > + fb_mmio = __request_region(hyperv_mmio, start, size, fb_mmio_name, 0); > } > > /** > -- > 2.37.1
