Skip to contentSkip to conversation

drivers:hv: Ensure that bridge windows don't overlap

PATCH Superseded Jake Oshins 3 participants 11 messages Apr 1, 2016 – Apr 5, 2016

Subsystems (3)

v4 of 5 Superseded Latest is v5 →

lore
Share & tools
lore
#0 lore raw

v4 cover drivers:hv: Ensure that bridge windows don't overlap

This series differs from v3 in that it folds in a patch suggested by the kbuild test robot, substituting resource_size() for directly calculating the size.

Hyper-V VMs expose paravirtual drivers through a mechanism called VMBus, which is managed by hv_vmbus.ko. For each parvirtual service instance, this driver exposes a new child device. Some of these child devices need memory address space, into which Hyper-V will map things like the virtual video frame buffer. This memory-mapped address space is chosen by the guest OS, not the hypervisor.

This is difficult to map onto the Linux pnp layer, as the code in the pnp layer to choose MMIO space keys off of bus type and it doesn't know anything about VMBus. The maintainers of the pnp layer have asked that we not offer patches to it that make it understand VMBus, but that we rather find ways of using the code in its current state. So hv_vmbus.ko exports a function, vmbus_allocate_mmio() for choosing the address space for any child driver that needs this facility.

The recently introduced PCI front-end driver for Hyper-V VMs (pci-hyperv.ko) uses vmbus_allocate_mmio() for choosing both the region of memory space into which real PCI Express devices are mapped. The regions allocated are made to look like root PCI bus bridge windows to the PCI driver, reusing all the code in the PCI driver for the rest of PCI device management.

The problem is that these bridge windows are marked in such a way that devices can still allocate from the memory space spanned by them, and this means that if two different PCI buses are created in the VM, each with devices under them, they may allocate the same memory space, leading to PCI Base Address Register which overlap.

This patch series fixes the problem by tracking allocations to child devices in a separate resource tree, marking them such that the bridge windows can't overlap. The main memory resource tree, iomem_resource, contains resources properly marked as bridge windows, allowing their children to overlap with them.

Jake Oshins (7):
drivers:hv: Lock access to hyperv_mmio resource tree drivers:hv: Make a function to free mmio regions through vmbus drivers:hv: Use new vmbus_mmio_free() from client drivers. drivers:hv: Reverse order of resources in hyperv_mmio drivers:hv: Track allocations of children of hv_vmbus in private
resource tree
drivers:hv: Record MMIO range in use by frame buffer drivers:hv: Separate out frame buffer logic when picking MMIO range

 drivers/hv/vmbus_drv.c          | 143 +++++++++++++++++++++++++++++-----------
 drivers/pci/host/pci-hyperv.c   |  14 ++--
 drivers/video/fbdev/hyperv_fb.c |   4 +-
 include/linux/hyperv.h          |   2 +-
 4 files changed, 115 insertions(+), 48 deletions(-)

--
1.9.1

#1 lore raw

PATCH v4 1/7 drivers:hv: Lock access to hyperv_mmio resource tree

In existing code, this tree of resources is created in single-threaded code and never modified after it is created, and thus needs no locking. This patch introduces a semaphore for tree access, as other patches in this series introduce run-time modifications of this resource tree which can happen on multiple threads.

---
 drivers/hv/vmbus_drv.c | 16 ++++++++++++----
 1 file changed, 12 insertions(+), 4 deletions(-)
drivers/hv/vmbus_drv.c +12 -4
index 64713ff..799518b 100644
@@ -102,6 +102,7 @@ static struct notifier_block hyperv_panic_block = {
 };
 
 struct resource *hyperv_mmio;
+DEFINE_SEMAPHORE(hyperv_mmio_lock);
 
 static int vmbus_exists(void)
 {
@@ -1132,7 +1133,10 @@ int vmbus_allocate_mmio(struct resource **new, struct hv_device *device_obj,
 	resource_size_t range_min, range_max, start, local_min, local_max;
 	const char *dev_n = dev_name(&device_obj->device);
 	u32 fb_end = screen_info.lfb_base + (screen_info.lfb_size << 1);
-	int i;
+	int i, retval;
+
+	retval = -ENXIO;
+	down(&hyperv_mmio_lock);
 
 	for (iter = hyperv_mmio; iter; iter = iter->sibling) {
 		if ((iter->start >= max) || (iter->end <= min))
@@ -1169,13 +1173,17 @@ int vmbus_allocate_mmio(struct resource **new, struct hv_device *device_obj,
 			for (; start + size - 1 <= local_max; start += align) {
 				*new = request_mem_region_exclusive(start, size,
 								    dev_n);
-				if (*new)
-					return 0;
+				if (*new) {
+					retval = 0;
+					goto exit;
+				}
 			}
 		}
 	}
 
-	return -ENXIO;
+exit:
+	up(&hyperv_mmio_lock);
+	return retval;
 }
 EXPORT_SYMBOL_GPL(vmbus_allocate_mmio);
 
Signature · 1 line
1.9.1
#2 lore raw

PATCH v4 2/7 drivers:hv: Make a function to free mmio regions through vmbus

In reply to Jake Oshins

This patch introduces a function that reverses everything done by vmbus_allocate_mmio(). Existing code just called release_mem_region(). Future patches in this series require a more complex sequence of actions, so this function is introduced to wrap those actions.

---
 drivers/hv/vmbus_drv.c | 15 +++++++++++++++
 include/linux/hyperv.h |  2 +-
 2 files changed, 16 insertions(+), 1 deletion(-)
drivers/hv/vmbus_drv.c +15 -0
index 799518b..60553c1 100644
@@ -1188,6 +1188,21 @@ exit:
 EXPORT_SYMBOL_GPL(vmbus_allocate_mmio);
 
 /**
+ * vmbus_free_mmio() - Free a memory-mapped I/O range.
+ * @start:		Base address of region to release.
+ * @size:		Size of the range to be allocated
+ *
+ * This function releases anything requested by
+ * vmbus_mmio_allocate().
+ */
+void vmbus_free_mmio(resource_size_t start, resource_size_t size)
+{
+	release_mem_region(start, size);
+
+}
+EXPORT_SYMBOL_GPL(vmbus_free_mmio);
+
+/**
  * vmbus_cpu_number_to_vp_number() - Map CPU to VP.
  * @cpu_number: CPU number in Linux terms
  *
include/linux/hyperv.h +1 -1
index aa0fadc..ecd81c3 100644
@@ -1091,7 +1091,7 @@ int vmbus_allocate_mmio(struct resource **new, struct hv_device *device_obj,
 			resource_size_t min, resource_size_t max,
 			resource_size_t size, resource_size_t align,
 			bool fb_overlap_ok);
-
+void vmbus_free_mmio(resource_size_t start, resource_size_t size);
 int vmbus_cpu_number_to_vp_number(int cpu_number);
 u64 hv_do_hypercall(u64 control, void *input, void *output);
 
Signature · 1 line
1.9.1
#3 lore raw

PATCH v4 3/7 drivers:hv: Use new vmbus_mmio_free() from client drivers.

In reply to Jake Oshins

This patch modifies all the callers of vmbus_mmio_allocate() to call vmbus_mmio_free() instead of release_mem_region().

---
 drivers/pci/host/pci-hyperv.c   | 14 +++++++-------
 drivers/video/fbdev/hyperv_fb.c |  4 ++--
 2 files changed, 9 insertions(+), 9 deletions(-)
drivers/pci/host/pci-hyperv.c +7 -7
index ed651ba..f2559b6 100644
@@ -1795,14 +1795,14 @@ static void hv_pci_free_bridge_windows(struct hv_pcibus_device *hbus)
 
 	if (hbus->low_mmio_space && hbus->low_mmio_res) {
 		hbus->low_mmio_res->flags |= IORESOURCE_BUSY;
-		release_mem_region(hbus->low_mmio_res->start,
-				   resource_size(hbus->low_mmio_res));
+		vmbus_free_mmio(hbus->low_mmio_res->start,
+				resource_size(hbus->low_mmio_res));
 	}
 
 	if (hbus->high_mmio_space && hbus->high_mmio_res) {
 		hbus->high_mmio_res->flags |= IORESOURCE_BUSY;
-		release_mem_region(hbus->high_mmio_res->start,
-				   resource_size(hbus->high_mmio_res));
+		vmbus_free_mmio(hbus->high_mmio_res->start,
+				resource_size(hbus->high_mmio_res));
 	}
 }
 
@@ -1880,8 +1880,8 @@ static int hv_pci_allocate_bridge_windows(struct hv_pcibus_device *hbus)
 
 release_low_mmio:
 	if (hbus->low_mmio_res) {
-		release_mem_region(hbus->low_mmio_res->start,
-				   resource_size(hbus->low_mmio_res));
+		vmbus_free_mmio(hbus->low_mmio_res->start,
+				resource_size(hbus->low_mmio_res));
 	}
 
 	return ret;
@@ -1924,7 +1924,7 @@ static int hv_allocate_config_window(struct hv_pcibus_device *hbus)
 
 static void hv_free_config_window(struct hv_pcibus_device *hbus)
 {
-	release_mem_region(hbus->mem_config->start, PCI_CONFIG_MMIO_LENGTH);
+	vmbus_free_mmio(hbus->mem_config->start, PCI_CONFIG_MMIO_LENGTH);
 }
 
 /**
drivers/video/fbdev/hyperv_fb.c +2 -2
index e2451bd..2fd49b2 100644
@@ -743,7 +743,7 @@ static int hvfb_getmem(struct hv_device *hdev, struct fb_info *info)
 err3:
 	iounmap(fb_virt);
 err2:
-	release_mem_region(par->mem->start, screen_fb_size);
+	vmbus_free_mmio(par->mem->start, screen_fb_size);
 	par->mem = NULL;
 err1:
 	if (!gen2vm)
@@ -758,7 +758,7 @@ static void hvfb_putmem(struct fb_info *info)
 	struct hvfb_par *par = info->par;
 
 	iounmap(info->screen_base);
-	release_mem_region(par->mem->start, screen_fb_size);
+	vmbus_free_mmio(par->mem->start, screen_fb_size);
 	par->mem = NULL;
 }
 
Signature · 1 line
1.9.1
Acked-by on v4 3/7
#4 lore raw

Re: [PATCH v4 3/7] drivers:hv: Use new vmbus_mmio_free() from client drivers.

Hi Jake,

Quoted 2 lines · Jake Oshins

This patch modifies all the callers of vmbus_mmio_allocate() to call vmbus_mmio_free() instead of release_mem_region().

This changelog merely restates the C code. Presumably there's some important difference between release_mem_region() and vmbus_mmio_free(), and we need a hint about what that is.

Oh, I see, there actually is no difference *yet*, but it's coming. I'd combine this with patch 2. Then the patch is obviously correct all by itself, and the changelog for patch 2 makes clear what's happening.

In changelogs, don't bother with "this patch does" or "this function is introduced." The context is obvious because the changelog is part of the commit. Write imperative sentences, e.g., "Call vmbus_mmio_free() instead of release_mem_region()."

Quoted 1 line

I think this is the only change that touches PCI, so I assume this series will be merged by somebody else.

Quoted 77 lines
---
 drivers/pci/host/pci-hyperv.c   | 14 +++++++-------
 drivers/video/fbdev/hyperv_fb.c |  4 ++--
 2 files changed, 9 insertions(+), 9 deletions(-)
drivers/pci/host/pci-hyperv.c +7 -7
index ed651ba..f2559b6 100644
@@ -1795,14 +1795,14 @@ static void hv_pci_free_bridge_windows(struct hv_pcibus_device *hbus)
 
 	if (hbus->low_mmio_space && hbus->low_mmio_res) {
 		hbus->low_mmio_res->flags |= IORESOURCE_BUSY;
-		release_mem_region(hbus->low_mmio_res->start,
-				   resource_size(hbus->low_mmio_res));
+		vmbus_free_mmio(hbus->low_mmio_res->start,
+				resource_size(hbus->low_mmio_res));
 	}
 
 	if (hbus->high_mmio_space && hbus->high_mmio_res) {
 		hbus->high_mmio_res->flags |= IORESOURCE_BUSY;
-		release_mem_region(hbus->high_mmio_res->start,
-				   resource_size(hbus->high_mmio_res));
+		vmbus_free_mmio(hbus->high_mmio_res->start,
+				resource_size(hbus->high_mmio_res));
 	}
 }
 
@@ -1880,8 +1880,8 @@ static int hv_pci_allocate_bridge_windows(struct hv_pcibus_device *hbus)
 
 release_low_mmio:
 	if (hbus->low_mmio_res) {
-		release_mem_region(hbus->low_mmio_res->start,
-				   resource_size(hbus->low_mmio_res));
+		vmbus_free_mmio(hbus->low_mmio_res->start,
+				resource_size(hbus->low_mmio_res));
 	}
 
 	return ret;
@@ -1924,7 +1924,7 @@ static int hv_allocate_config_window(struct hv_pcibus_device *hbus)
 
 static void hv_free_config_window(struct hv_pcibus_device *hbus)
 {
-	release_mem_region(hbus->mem_config->start, PCI_CONFIG_MMIO_LENGTH);
+	vmbus_free_mmio(hbus->mem_config->start, PCI_CONFIG_MMIO_LENGTH);
 }
 
 /**
drivers/video/fbdev/hyperv_fb.c +2 -2
index e2451bd..2fd49b2 100644
@@ -743,7 +743,7 @@ static int hvfb_getmem(struct hv_device *hdev, struct fb_info *info)
 err3:
 	iounmap(fb_virt);
 err2:
-	release_mem_region(par->mem->start, screen_fb_size);
+	vmbus_free_mmio(par->mem->start, screen_fb_size);
 	par->mem = NULL;
 err1:
 	if (!gen2vm)
@@ -758,7 +758,7 @@ static void hvfb_putmem(struct fb_info *info)
 	struct hvfb_par *par = info->par;
 
 	iounmap(info->screen_base);
-	release_mem_region(par->mem->start, screen_fb_size);
+	vmbus_free_mmio(par->mem->start, screen_fb_size);
 	par->mem = NULL;
 }
 
Signature · 6 lines
1.9.1

--
To unsubscribe from this list: send the line "unsubscribe linux-pci" in
the body of a message to [email protected]
More majordomo info at  http://vger.kernel.org/majordomo-info.html
#5 lore raw

RE: [PATCH v4 3/7] drivers:hv: Use new vmbus_mmio_free() from client drivers.

Quoted 34 lines

-----Original Message-----
From: Bjorn Helgaas [mailto:[email protected]]
Sent: Tuesday, April 5, 2016 11:00 AM
To: Jake Oshins <[email protected]>
Cc: [email protected]; [email protected]; KY Srinivasan <[email protected]>; [email protected]; [email protected]; [email protected]; [email protected]; [email protected]; Haiyang Zhang <[email protected]>; Hadden Hoppert <[email protected]>
Subject: Re: [PATCH v4 3/7] drivers:hv: Use new vmbus_mmio_free() from client drivers.

Hi Jake,

Quoted 2 lines · Jake Oshins

This patch modifies all the callers of vmbus_mmio_allocate() to call vmbus_mmio_free() instead of release_mem_region().

This changelog merely restates the C code. Presumably there's some important difference between release_mem_region() and vmbus_mmio_free(), and we need a hint about what that is.

Oh, I see, there actually is no difference *yet*, but it's coming. I'd combine this with patch 2. Then the patch is obviously correct all by itself, and the changelog for patch 2 makes clear what's happening.

In changelogs, don't bother with "this patch does" or "this function is introduced." The context is obvious because the changelog is part of the commit. Write imperative sentences, e.g., "Call vmbus_mmio_free() instead of release_mem_region()."

Quoted 6 lines
Quoted 1 line

I think this is the only change that touches PCI, so I assume this series will be merged by somebody else.

I am hoping this will go through Greg's tree.

K. Y

Quoted 90 lines

drivers/pci/host/pci-hyperv.c static void hv_pci_free_bridge_windows(struct

Quoted 11 lines
---
 drivers/pci/host/pci-hyperv.c   | 14 +++++++-------
 drivers/video/fbdev/hyperv_fb.c |  4 ++--
 2 files changed, 9 insertions(+), 9 deletions(-)
drivers/pci/host/pci-hyperv.c +0 -0
index ed651ba..f2559b6 100644
@@ -1795,14 +1795,14 @@ static void hv_pci_free_bridge_windows(struct

hv_pcibus_device *hbus)

drivers/pci/host/pci-hyperv.c static int hv_pci_allocate_bridge_windows(struct

Quoted 19 lines
drivers/pci/host/pci-hyperv.c +4 -4
 	if (hbus->low_mmio_space && hbus->low_mmio_res) {
 		hbus->low_mmio_res->flags |= IORESOURCE_BUSY;
-		release_mem_region(hbus->low_mmio_res->start,
-				   resource_size(hbus->low_mmio_res));
+		vmbus_free_mmio(hbus->low_mmio_res->start,
+				resource_size(hbus->low_mmio_res));
 	}
 
 	if (hbus->high_mmio_space && hbus->high_mmio_res) {
 		hbus->high_mmio_res->flags |= IORESOURCE_BUSY;
-		release_mem_region(hbus->high_mmio_res->start,
-				   resource_size(hbus->high_mmio_res));
+		vmbus_free_mmio(hbus->high_mmio_res->start,
+				resource_size(hbus->high_mmio_res));
 	}
 }
 
@@ -1880,8 +1880,8 @@ static int hv_pci_allocate_bridge_windows(struct

hv_pcibus_device *hbus)

drivers/pci/host/pci-hyperv.c static int hv_allocate_config_window(struct

Quoted 11 lines
drivers/pci/host/pci-hyperv.c +2 -2
 release_low_mmio:
 	if (hbus->low_mmio_res) {
-		release_mem_region(hbus->low_mmio_res->start,
-				   resource_size(hbus->low_mmio_res));
+		vmbus_free_mmio(hbus->low_mmio_res->start,
+				resource_size(hbus->low_mmio_res));
 	}
 
 	return ret;
@@ -1924,7 +1924,7 @@ static int hv_allocate_config_window(struct

hv_pcibus_device *hbus)

drivers/pci/host/pci-hyperv.c static int hv_allocate_config_window(struct hv_pcibus_device *hbus)

Quoted 4 lines
drivers/pci/host/pci-hyperv.c +0 -1
 static void hv_free_config_window(struct hv_pcibus_device *hbus)
 {
-	release_mem_region(hbus->mem_config->start,

PCI_CONFIG_MMIO_LENGTH);

drivers/pci/host/pci-hyperv.c static int hv_allocate_config_window(struct · line 1926

Quoted 1 line
drivers/pci/host/pci-hyperv.c +1 -0
+	vmbus_free_mmio(hbus->mem_config->start,

PCI_CONFIG_MMIO_LENGTH);

Review

Quoted 4 lines
drivers/pci/host/pci-hyperv.c +0 -0
 }
 
 /**
(quoted diff) +0 -0

b/drivers/video/fbdev/hyperv_fb.c

drivers/video/fbdev/hyperv_fb.c static int hvfb_getmem(struct hv_device *hdev,

Quoted 4 lines

index e2451bd..2fd49b2 100644

drivers/video/fbdev/hyperv_fb.c +0 -0
@@ -743,7 +743,7 @@ static int hvfb_getmem(struct hv_device *hdev,

struct fb_info *info)

Quoted 20 lines
drivers/video/fbdev/hyperv_fb.c +2 -3
 err3:
 	iounmap(fb_virt);
 err2:
-	release_mem_region(par->mem->start, screen_fb_size);
+	vmbus_free_mmio(par->mem->start, screen_fb_size);
 	par->mem = NULL;
 err1:
 	if (!gen2vm)
@@ -758,7 +758,7 @@ static void hvfb_putmem(struct fb_info *info)
 	struct hvfb_par *par = info->par;
 
 	iounmap(info->screen_base);
-	release_mem_region(par->mem->start, screen_fb_size);
+	vmbus_free_mmio(par->mem->start, screen_fb_size);
 	par->mem = NULL;
 }
 
--

1.9.1

Quoted 4 lines

--
To unsubscribe from this list: send the line "unsubscribe linux-pci" in the body of a message to [email protected]
More majordomo info at

https://na01.safelinks.protection.outlook.com/?url=http%3a%2f%2fvger.ker
nel.org%2fmajordomo- info.html&data=01%7c01%7ckys%40microsoft.com%7cf391db0d7312429368e 908d35d7c17ed%7c72f988bf86f141af91ab2d7cd011db47%7c1&sdata=bH6jnta OKCSn%2fZw%2bCN1P9wV9%2bRk0om8hw4evDezjR0Q%3d

#6 lore raw

RE: [PATCH v4 3/7] drivers:hv: Use new vmbus_mmio_free() from client drivers.

In reply to Bjorn Helgaas
Quoted 27 lines

-----Original Message-----
From: Bjorn Helgaas [mailto:[email protected]]
Sent: Tuesday, April 5, 2016 11:00 AM
To: Jake Oshins <[email protected]>
Cc: [email protected]; [email protected]; KY Srinivasan <[email protected]>; [email protected]; [email protected]; [email protected]; [email protected]; [email protected]; Haiyang Zhang <[email protected]>; Hadden Hoppert <[email protected]>
Subject: Re: [PATCH v4 3/7] drivers:hv: Use new vmbus_mmio_free() from client drivers.

Hi Jake,

Quoted 2 lines · Jake Oshins

This patch modifies all the callers of vmbus_mmio_allocate() to call vmbus_mmio_free() instead of release_mem_region().

This changelog merely restates the C code. Presumably there's some important difference between release_mem_region() and vmbus_mmio_free(), and we need a hint about what that is.

Oh, I see, there actually is no difference *yet*, but it's coming. I'd combine this with patch 2. Then the patch is obviously correct all by itself, and the changelog for patch 2 makes clear what's happening.

Quoted 5 lines

In changelogs, don't bother with "this patch does" or "this function is introduced." The context is obvious because the changelog is part of the commit. Write imperative sentences, e.g., "Call vmbus_mmio_free() instead of release_mem_region()."

Will do. Thanks.

-- Jake Oshins

#7 lore raw

PATCH v4 4/7 drivers:hv: Reverse order of resources in hyperv_mmio

In reply to Jake Oshins

A patch later in this series allocates child nodes in this resource tree. For that to work, this tree needs to be sorted in ascending order.

---
 drivers/hv/vmbus_drv.c | 3 +--
 1 file changed, 1 insertion(+), 2 deletions(-)
drivers/hv/vmbus_drv.c +1 -2
index 60553c1..1ce47d0 100644
@@ -1049,7 +1049,6 @@ static acpi_status vmbus_walk_resources(struct acpi_resource *res, void *ctx)
 	new_res->end = end;
 
 	/*
-	 * Stick ranges from higher in address space at the front of the list.
 	 * If two ranges are adjacent, merge them.
 	 */
 	do {
@@ -1070,7 +1069,7 @@ static acpi_status vmbus_walk_resources(struct acpi_resource *res, void *ctx)
 			break;
 		}
 
-		if ((*old_res)->end < new_res->start) {
+		if ((*old_res)->start > new_res->end) {
 			new_res->sibling = *old_res;
 			if (prev_res)
 				(*prev_res)->sibling = new_res;
Signature · 1 line
1.9.1
#8 lore raw

PATCH v4 5/7 drivers:hv: Track allocations of children of hv_vmbus in private resource tree

In reply to Jake Oshins

This patch changes vmbus_allocate_mmio() and vmbus_free_mmio() so that when child paravirtual devices allocate memory-mapped I/O space, they allocate it privately from a resource tree pointed at by hyperv_mmio and also by the public resource tree iomem_resource. This allows the region to be marked as "busy" in the private tree, but a "bridge window" in the public tree, guaranteeing that no two bridge windows will overlap each other but while also allowing the PCI device children of the bridge windows to overlap that window.

One might conclude that this belongs in the pnp layer, rather than in this driver. Rafael Wysocki, the maintainter of the pnp layer, has previously asked that we not modify the pnp layer as it is considered deprecated. This patch is thus essentially a workaround.

---
 drivers/hv/vmbus_drv.c | 22 +++++++++++++++++++++-
 1 file changed, 21 insertions(+), 1 deletion(-)
drivers/hv/vmbus_drv.c +21 -1
index 1ce47d0..dfc6149 100644
@@ -1128,7 +1128,7 @@ int vmbus_allocate_mmio(struct resource **new, struct hv_device *device_obj,
 			resource_size_t size, resource_size_t align,
 			bool fb_overlap_ok)
 {
-	struct resource *iter;
+	struct resource *iter, *shadow;
 	resource_size_t range_min, range_max, start, local_min, local_max;
 	const char *dev_n = dev_name(&device_obj->device);
 	u32 fb_end = screen_info.lfb_base + (screen_info.lfb_size << 1);
@@ -1170,12 +1170,22 @@ int vmbus_allocate_mmio(struct resource **new, struct hv_device *device_obj,
 
 			start = (local_min + align - 1) & ~(align - 1);
 			for (; start + size - 1 <= local_max; start += align) {
+				shadow = __request_region(iter, start,
+							  size,
+							  NULL,
+							  IORESOURCE_BUSY);
+				if (!shadow)
+					continue;
+
 				*new = request_mem_region_exclusive(start, size,
 								    dev_n);
 				if (*new) {
+					shadow->name = (char *)*new;
 					retval = 0;
 					goto exit;
 				}
+
+				__release_region(iter, start, size);
 			}
 		}
 	}
@@ -1196,7 +1206,17 @@ EXPORT_SYMBOL_GPL(vmbus_allocate_mmio);
  */
 void vmbus_free_mmio(resource_size_t start, resource_size_t size)
 {
+	struct resource *iter;
+
+	down(&hyperv_mmio_lock);
+	for (iter = hyperv_mmio; iter; iter = iter->sibling) {
+		if ((iter->start >= start + size) || (iter->end <= start))
+			continue;
+
+		__release_region(iter, start, size);
+	}
 	release_mem_region(start, size);
+	up(&hyperv_mmio_lock);
 
 }
 EXPORT_SYMBOL_GPL(vmbus_free_mmio);
Signature · 1 line
1.9.1
#9 lore raw

PATCH v4 6/7 drivers:hv: Record MMIO range in use by frame buffer

In reply to Jake Oshins

Later in the boot sequence, we need to figure out which memory ranges can be given out to various paravirtual drivers. The hyperv_fb driver should, ideally, be placed right on top of the frame buffer, without some other device getting plopped on top of this range in the meantime. Recording this now allows that to be guaranteed.

---
 drivers/hv/vmbus_drv.c | 37 ++++++++++++++++++++++++++++++++++++-
 1 file changed, 36 insertions(+), 1 deletion(-)
drivers/hv/vmbus_drv.c +36 -1
index dfc6149..eaa5c3b 100644
@@ -41,6 +41,7 @@
 #include <linux/ptrace.h>
 #include <linux/screen_info.h>
 #include <linux/kdebug.h>
+#include <linux/efi.h>
 #include "hyperv_vmbus.h"
 
 static struct acpi_device  *hv_acpi_dev;
@@ -101,6 +102,8 @@ static struct notifier_block hyperv_panic_block = {
 	.notifier_call = hyperv_panic_event,
 };
 
+static const char *fb_mmio_name = "fb_range";
+static struct resource *fb_mmio;
 struct resource *hyperv_mmio;
 DEFINE_SEMAPHORE(hyperv_mmio_lock);
 
@@ -1091,6 +1094,12 @@ static int vmbus_acpi_remove(struct acpi_device *device)
 	struct resource *next_res;
 
 	if (hyperv_mmio) {
+		if (fb_mmio) {
+			__release_region(hyperv_mmio, fb_mmio->start,
+					 resource_size(fb_mmio));
+			fb_mmio = NULL;
+		}
+
 		for (cur_res = hyperv_mmio; cur_res; cur_res = next_res) {
 			next_res = cur_res->sibling;
 			kfree(cur_res);
@@ -1100,6 +1109,30 @@ static int vmbus_acpi_remove(struct acpi_device *device)
 	return 0;
 }
 
+static void vmbus_reserve_fb(void)
+{
+	int size;
+	/*
+	 * 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);
+		}
+	}
+}
+
 /**
  * vmbus_allocate_mmio() - Pick a memory-mapped I/O range.
  * @new:		If successful, supplied a pointer to the
@@ -1261,8 +1294,10 @@ static int vmbus_acpi_add(struct acpi_device *device)
 
 		if (ACPI_FAILURE(result))
 			continue;
-		if (hyperv_mmio)
+		if (hyperv_mmio) {
+			vmbus_reserve_fb();
 			break;
+		}
 	}
 	ret_val = 0;
 
Signature · 1 line
1.9.1
#10 lore raw

PATCH v4 7/7 drivers:hv: Separate out frame buffer logic when picking MMIO range

In reply to Jake Oshins

Simplify the logic that picks MMIO ranges by pulling out the logic related to trying to lay frame buffer claim on top of where the firmware placed the frame buffer.

---
 drivers/hv/vmbus_drv.c | 80 ++++++++++++++++++++++----------------------------
 1 file changed, 35 insertions(+), 45 deletions(-)
drivers/hv/vmbus_drv.c +35 -45
index eaa5c3b..a29a6c0 100644
@@ -1162,64 +1162,54 @@ int vmbus_allocate_mmio(struct resource **new, struct hv_device *device_obj,
 			bool fb_overlap_ok)
 {
 	struct resource *iter, *shadow;
-	resource_size_t range_min, range_max, start, local_min, local_max;
+	resource_size_t range_min, range_max, start;
 	const char *dev_n = dev_name(&device_obj->device);
-	u32 fb_end = screen_info.lfb_base + (screen_info.lfb_size << 1);
-	int i, retval;
+	int retval;
 
 	retval = -ENXIO;
 	down(&hyperv_mmio_lock);
 
+	/*
+	 * If overlaps with frame buffers are allowed, then first attempt to
+	 * make the allocation from within the reserved region.  Because it
+	 * is already reserved, no shadow allocation is necessary.
+	 */
+	if (fb_overlap_ok && fb_mmio && !(min > fb_mmio->end) &&
+	    !(max < fb_mmio->start)) {
+
+		range_min = fb_mmio->start;
+		range_max = fb_mmio->end;
+		start = (range_min + align - 1) & ~(align - 1);
+		for (; start + size - 1 <= range_max; start += align) {
+			*new = request_mem_region_exclusive(start, size, dev_n);
+			if (*new) {
+				retval = 0;
+				goto exit;
+			}
+		}
+	}
+
 	for (iter = hyperv_mmio; iter; iter = iter->sibling) {
 		if ((iter->start >= max) || (iter->end <= min))
 			continue;
 
 		range_min = iter->start;
 		range_max = iter->end;
-
-		/* If this range overlaps the frame buffer, split it into
-		   two tries. */
-		for (i = 0; i < 2; i++) {
-			local_min = range_min;
-			local_max = range_max;
-			if (fb_overlap_ok || (range_min >= fb_end) ||
-			    (range_max <= screen_info.lfb_base)) {
-				i++;
-			} else {
-				if ((range_min <= screen_info.lfb_base) &&
-				    (range_max >= screen_info.lfb_base)) {
-					/*
-					 * The frame buffer is in this window,
-					 * so trim this into the part that
-					 * preceeds the frame buffer.
-					 */
-					local_max = screen_info.lfb_base - 1;
-					range_min = fb_end;
-				} else {
-					range_min = fb_end;
-					continue;
-				}
+		start = (range_min + align - 1) & ~(align - 1);
+		for (; start + size - 1 <= range_max; start += align) {
+			shadow = __request_region(iter, start, size, NULL,
+						  IORESOURCE_BUSY);
+			if (!shadow)
+				continue;
+
+			*new = request_mem_region_exclusive(start, size, dev_n);
+			if (*new) {
+				shadow->name = (char *)*new;
+				retval = 0;
+				goto exit;
 			}
 
-			start = (local_min + align - 1) & ~(align - 1);
-			for (; start + size - 1 <= local_max; start += align) {
-				shadow = __request_region(iter, start,
-							  size,
-							  NULL,
-							  IORESOURCE_BUSY);
-				if (!shadow)
-					continue;
-
-				*new = request_mem_region_exclusive(start, size,
-								    dev_n);
-				if (*new) {
-					shadow->name = (char *)*new;
-					retval = 0;
-					goto exit;
-				}
-
-				__release_region(iter, start, size);
-			}
+			__release_region(iter, start, size);
 		}
 	}
 
Signature · 1 line
1.9.1