| Message ID | 20220825090024.1007883-3-vkuznets@redhat.com (mailing list archive) |
|---|---|
| State | New, archived |
| 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 25, 2022 2:00 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 Just so I'm clear, by "EFI fb" you mean the EFI layer code that sets up the frame buffer before the Linux kernel ever boots, right? You are not referring to the Linux kernel EFI framebuffer driver, which may or may not be configured in the kernel. > (or, in some edge cases like kexec, the address where the buffer was > moved, see https://lore.kernel.org/all/20201014092429.1415040-1-kasong@redhat.com/ > but on Gen1 VM it depends on bootloader behavior. With grub, it depends > on 'gfxpayload=' setting but in some cases it is observed to be zero. > Relying on 'screen_info.lfb_base' to reserve framebuffer region is > risky. Instead, it is possible to get the address from the dedicated > PCI device which is always present. > > Check for legacy PCI video device presence and reserve the whole > region for framebuffer on Gen1 VMs. > > Signed-off-by: Vitaly Kuznetsov <vkuznets@redhat.com> > --- > drivers/hv/vmbus_drv.c | 46 +++++++++++++++++++++++++++++------------- > 1 file changed, 32 insertions(+), 14 deletions(-) > > diff --git a/drivers/hv/vmbus_drv.c b/drivers/hv/vmbus_drv.c > index 23c680d1a0f5..536f68e563c6 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" > > @@ -2262,26 +2263,43 @@ 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) { > + 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 Reviewed-by: Michael Kelley <mikelley@microsoft.com>
"Michael Kelley (LINUX)" <mikelley@microsoft.com> writes: > From: Vitaly Kuznetsov <vkuznets@redhat.com> Sent: Thursday, August 25, 2022 2:00 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 > > Just so I'm clear, by "EFI fb" you mean the EFI layer code that sets > up the frame buffer before the Linux kernel ever boots, right? > You are not referring to the Linux kernel EFI framebuffer > driver, which may or may not be configured in the kernel. My very shallow understanding is that initially, screen_info comes from boot_params and this depends on how Linux was booted. Kernel EFI framebuffer (when enabled), however, gets it first and can modify it (see efifb_setup()) before we get to analyze it in Vmbus. > >> (or, in some edge cases like kexec, the address where the buffer was >> moved, see https://lore.kernel.org/all/20201014092429.1415040-1-kasong@redhat.com/ >> but on Gen1 VM it depends on bootloader behavior. With grub, it depends >> on 'gfxpayload=' setting but in some cases it is observed to be zero. >> Relying on 'screen_info.lfb_base' to reserve framebuffer region is >> risky. Instead, it is possible to get the address from the dedicated >> PCI device which is always present. >> >> Check for legacy PCI video device presence and reserve the whole >> region for framebuffer on Gen1 VMs. >> >> Signed-off-by: Vitaly Kuznetsov <vkuznets@redhat.com> >> --- >> drivers/hv/vmbus_drv.c | 46 +++++++++++++++++++++++++++++------------- >> 1 file changed, 32 insertions(+), 14 deletions(-) >> >> diff --git a/drivers/hv/vmbus_drv.c b/drivers/hv/vmbus_drv.c >> index 23c680d1a0f5..536f68e563c6 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" >> >> @@ -2262,26 +2263,43 @@ 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) { >> + 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 > > Reviewed-by: Michael Kelley <mikelley@microsoft.com> > Thanks!
diff --git a/drivers/hv/vmbus_drv.c b/drivers/hv/vmbus_drv.c index 23c680d1a0f5..536f68e563c6 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" @@ -2262,26 +2263,43 @@ 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) { + 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); } /**
vmbus_reserve_fb() tries reserving framebuffer region iff 'screen_info.lfb_base' is set. Gen2 VMs seem to have it set by EFI fb (or, in some edge cases like kexec, the address where the buffer was moved, see https://lore.kernel.org/all/20201014092429.1415040-1-kasong@redhat.com/) but on Gen1 VM it depends on bootloader behavior. With grub, it depends on 'gfxpayload=' setting but in some cases it is observed to be zero. Relying on 'screen_info.lfb_base' to reserve framebuffer region is risky. Instead, it is possible to get the address from the dedicated PCI device which is always present. Check for legacy PCI video device presence and reserve the whole region for framebuffer on Gen1 VMs. Signed-off-by: Vitaly Kuznetsov <vkuznets@redhat.com> --- drivers/hv/vmbus_drv.c | 46 +++++++++++++++++++++++++++++------------- 1 file changed, 32 insertions(+), 14 deletions(-)