Changes since v2:
- Add Michael's R-b tags to PATCHes1-3 and Bjorn's A-b tag to PATCH1.
- Commit messages tweaks [Michael].
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]
...
Recently merged commit a0ab5abced55 ("drm/hyperv : Removing the restruction of
VRAM allocation with PCI bar size") improved the situation as resources,
reserved through vmbus_allocate_mmio() can't be allocated twice:
...
f8000000-fffbffff : PCI Bus 0000:00
f8000000-fbffffff : 0000:00:08.0
f8000000-f8001fff : bb8c4f33-2ba2-4808-9f7f-02f3b4da22fe
f8100000-f88fffff : 5620e0c7-8062-4dce-aeb7-520c7ef76171
...
Always reserve FB region on Gen1 VMs (PATCH2) and make sure we never allocate
anything besides framebuffer from there (PATCH3).
Vitaly Kuznetsov (3):
PCI: Move PCI_VENDOR_ID_MICROSOFT/PCI_DEVICE_ID_HYPERV_VIDEO
definitions to pci_ids.h
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 | 3 -
drivers/hv/vmbus_drv.c | 56 ++++++++++++++-----
.../net/ethernet/microsoft/mana/gdma_main.c | 4 --
drivers/video/fbdev/hyperv_fb.c | 4 --
include/linux/pci_ids.h | 3 +
5 files changed, 44 insertions(+), 26 deletions(-)
--
2.37.1
There are already three places in kernel which define PCI_VENDOR_ID_MICROSOFT
and two for PCI_DEVICE_ID_HYPERV_VIDEO and there's a need to use these
from core Vmbus code. Move the defines where they belong.
No functional change.
Reviewed-by: Michael Kelley <[email protected]>
Acked-by: Bjorn Helgaas <[email protected]> # pci_ids.h
Signed-off-by: Vitaly Kuznetsov <[email protected]>
---
drivers/gpu/drm/hyperv/hyperv_drm_drv.c | 3 ---
drivers/net/ethernet/microsoft/mana/gdma_main.c | 4 ----
drivers/video/fbdev/hyperv_fb.c | 4 ----
include/linux/pci_ids.h | 3 +++
4 files changed, 3 insertions(+), 11 deletions(-)
diff --git a/drivers/gpu/drm/hyperv/hyperv_drm_drv.c b/drivers/gpu/drm/hyperv/hyperv_drm_drv.c
index 6d11e7938c83..40888e36f91a 100644
--- a/drivers/gpu/drm/hyperv/hyperv_drm_drv.c
+++ b/drivers/gpu/drm/hyperv/hyperv_drm_drv.c
@@ -23,9 +23,6 @@
#define DRIVER_MAJOR 1
#define DRIVER_MINOR 0
-#define PCI_VENDOR_ID_MICROSOFT 0x1414
-#define PCI_DEVICE_ID_HYPERV_VIDEO 0x5353
-
DEFINE_DRM_GEM_FOPS(hv_fops);
static struct drm_driver hyperv_driver = {
diff --git a/drivers/net/ethernet/microsoft/mana/gdma_main.c b/drivers/net/ethernet/microsoft/mana/gdma_main.c
index 5f9240182351..00d8198072ae 100644
--- a/drivers/net/ethernet/microsoft/mana/gdma_main.c
+++ b/drivers/net/ethernet/microsoft/mana/gdma_main.c
@@ -1465,10 +1465,6 @@ static void mana_gd_shutdown(struct pci_dev *pdev)
pci_disable_device(pdev);
}
-#ifndef PCI_VENDOR_ID_MICROSOFT
-#define PCI_VENDOR_ID_MICROSOFT 0x1414
-#endif
-
static const struct pci_device_id mana_id_table[] = {
{ PCI_DEVICE(PCI_VENDOR_ID_MICROSOFT, MANA_PF_DEVICE_ID) },
{ PCI_DEVICE(PCI_VENDOR_ID_MICROSOFT, MANA_VF_DEVICE_ID) },
diff --git a/drivers/video/fbdev/hyperv_fb.c b/drivers/video/fbdev/hyperv_fb.c
index 886c564787f1..b58b445bb529 100644
--- a/drivers/video/fbdev/hyperv_fb.c
+++ b/drivers/video/fbdev/hyperv_fb.c
@@ -74,10 +74,6 @@
#define SYNTHVID_DEPTH_WIN8 32
#define SYNTHVID_FB_SIZE_WIN8 (8 * 1024 * 1024)
-#define PCI_VENDOR_ID_MICROSOFT 0x1414
-#define PCI_DEVICE_ID_HYPERV_VIDEO 0x5353
-
-
enum pipe_msg_type {
PIPE_MSG_INVALID,
PIPE_MSG_DATA,
diff --git a/include/linux/pci_ids.h b/include/linux/pci_ids.h
index 6feade66efdb..15b49e655ce3 100644
--- a/include/linux/pci_ids.h
+++ b/include/linux/pci_ids.h
@@ -2079,6 +2079,9 @@
#define PCI_DEVICE_ID_ICE_1712 0x1712
#define PCI_DEVICE_ID_VT1724 0x1724
+#define PCI_VENDOR_ID_MICROSOFT 0x1414
+#define PCI_DEVICE_ID_HYPERV_VIDEO 0x5353
+
#define PCI_VENDOR_ID_OXSEMI 0x1415
#define PCI_DEVICE_ID_OXSEMI_12PCI840 0x8403
#define PCI_DEVICE_ID_OXSEMI_PCIe840 0xC000
--
2.37.1
vmbus_reserve_fb() tries reserving framebuffer region iff
'screen_info.lfb_base' is set. Gen2 VMs seem to have it set by EFI
and/or by the kernel EFI FB driver (or, in some edge cases like kexec,
the address where the buffer was moved, see
https://lore.kernel.org/all/[email protected]/)
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.
That being said, relying on 'screen_info.lfb_base' to reserve
framebuffer region is risky. For Gen1 VMs, it should always be
possible to get the address from the dedicated PCI device instead.
Check for legacy PCI video device presence and reserve the whole
region for framebuffer on Gen1 VMs.
Reviewed-by: Michael Kelley <[email protected]>
Signed-off-by: Vitaly Kuznetsov <[email protected]>
---
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
On Sat, Aug 27, 2022 at 03:03:43PM +0200, Vitaly Kuznetsov wrote:
> There are already three places in kernel which define PCI_VENDOR_ID_MICROSOFT
> and two for PCI_DEVICE_ID_HYPERV_VIDEO and there's a need to use these
> from core Vmbus code. Move the defines where they belong.
It's a minor annoyance that the above is 81 characters long when "git
log" adds its 4-character indent, so it wraps in a default terminal.
It'd be nice if we could settle on a conventional spelling of "Vmbus",
too. "Vmbus" looks to be in the minority:
$ git grep Vmbus | wc -l; git grep VMbus | wc -l; git grep VMBus | wc -l
4
82
62
FWIW, one published microsoft.com doc uses "VMBus":
https://docs.microsoft.com/en-us/virtualization/hyper-v-on-windows/reference/hyper-v-architecture
Bjorn Helgaas <[email protected]> writes:
> On Sat, Aug 27, 2022 at 03:03:43PM +0200, Vitaly Kuznetsov wrote:
>> There are already three places in kernel which define PCI_VENDOR_ID_MICROSOFT
>> and two for PCI_DEVICE_ID_HYPERV_VIDEO and there's a need to use these
>> from core Vmbus code. Move the defines where they belong.
>
> It's a minor annoyance that the above is 81 characters long when "git
> log" adds its 4-character indent, so it wraps in a default terminal.
>
> It'd be nice if we could settle on a conventional spelling of "Vmbus",
> too. "Vmbus" looks to be in the minority:
>
> $ git grep Vmbus | wc -l; git grep VMbus | wc -l; git grep VMBus | wc -l
> 4
> 82
> 62
>
> FWIW, one published microsoft.com doc uses "VMBus":
> https://docs.microsoft.com/en-us/virtualization/hyper-v-on-windows/reference/hyper-v-architecture
Makes sense,
Wei,
assuming there are no other concerns about these patches, would you be
able to tweak the commit message here when queueing or would you like me
to send v4 instead?
Thanks!
--
Vitaly
On Sat, Aug 27, 2022 at 03:03:42PM +0200, Vitaly Kuznetsov wrote:
[...]
> Changes since v2re (PATCH3).
>
> Vitaly Kuznetsov (3):
> PCI: Move PCI_VENDOR_ID_MICROSOFT/PCI_DEVICE_ID_HYPERV_VIDEO
> definitions to pci_ids.h
> Drivers: hv: Always reserve framebuffer region for Gen1 VMs
> Drivers: hv: Never allocate anything besides framebuffer from
> framebuffer memory region
Series applied to hyperv-fixes. Thanks.
>
On Tue, Aug 30, 2022 at 09:31:54AM +0200, Vitaly Kuznetsov wrote:
> Bjorn Helgaas <[email protected]> writes:
>
> > On Sat, Aug 27, 2022 at 03:03:43PM +0200, Vitaly Kuznetsov wrote:
> >> There are already three places in kernel which define PCI_VENDOR_ID_MICROSOFT
> >> and two for PCI_DEVICE_ID_HYPERV_VIDEO and there's a need to use these
> >> from core Vmbus code. Move the defines where they belong.
> >
> > It's a minor annoyance that the above is 81 characters long when "git
> > log" adds its 4-character indent, so it wraps in a default terminal.
> >
> > It'd be nice if we could settle on a conventional spelling of "Vmbus",
> > too. "Vmbus" looks to be in the minority:
> >
> > $ git grep Vmbus | wc -l; git grep VMbus | wc -l; git grep VMBus | wc -l
> > 4
> > 82
> > 62
> >
> > FWIW, one published microsoft.com doc uses "VMBus":
> > https://docs.microsoft.com/en-us/virtualization/hyper-v-on-windows/reference/hyper-v-architecture
>
> Makes sense,
>
> Wei,
>
> assuming there are no other concerns about these patches, would you be
> able to tweak the commit message here when queueing or would you like me
> to send v4 instead?
I can do the tweaking. Don't bother sending v4 if there is no other
concern.
Thanks,
Wei.
>
> Thanks!
>
> --
> Vitaly
>