WARNING - OLD ARCHIVES

This is an archived copy of the Xen.org mailing list, which we have preserved to ensure that existing links to archives are not broken. The live archive, which contains the latest emails, can be found at http://lists.xen.org/
   
 
 
Xen 
 
Home Products Support Community News
 
   
 

xen-devel

Re: [Xen-devel] [PATCH] dom0 linux: Reassign memory resources to device

To: "Yuji Shimada <shimada-yxb@xxxxxxxxxxxxxxx>; Keir Fraser" <keir.fraser@xxxxxxxxxx>
Subject: Re: [Xen-devel] [PATCH] dom0 linux: Reassign memory resources to device for pci passthrough.
From: "Zhao, Yu" <yu.zhao@xxxxxxxxx>
Date: Fri, 10 Oct 2008 15:05:29 +0800
Cc: "xen-devel@xxxxxxxxxxxxxxxxxxx" <xen-devel@xxxxxxxxxxxxxxxxxxx>
Delivery-date: Fri, 10 Oct 2008 00:06:38 -0700
Envelope-to: www-data@xxxxxxxxxxxxxxxxxxx
In-reply-to: <20081009083706.BF13.SHIMADA-YXB@xxxxxxxxxxxxxxx>
List-help: <mailto:xen-devel-request@lists.xensource.com?subject=help>
List-id: Xen developer discussion <xen-devel.lists.xensource.com>
List-post: <mailto:xen-devel@lists.xensource.com>
List-subscribe: <http://lists.xensource.com/mailman/listinfo/xen-devel>, <mailto:xen-devel-request@lists.xensource.com?subject=subscribe>
List-unsubscribe: <http://lists.xensource.com/mailman/listinfo/xen-devel>, <mailto:xen-devel-request@lists.xensource.com?subject=unsubscribe>
References: <20081009083706.BF13.SHIMADA-YXB@xxxxxxxxxxxxxxx>
Sender: xen-devel-bounces@xxxxxxxxxxxxxxxxxxx
User-agent: Thunderbird 2.0.0.17 (Windows/20080914)
Yuji & Keir,

I guess the comments I gave yesterday is obscure, so I wrote up more details this time. Please take a look.
Thanks,
Yu

> To reassign resources, please add boot parameters of dom0 linux as follows.
>
>     reassign_resources reassigndev=00:1d.7,01:00.0
>
>         reassign_resources
>                         Enables reassigning resources.
>
>         reassigndev=    Specifies devices include I/O device and PCI-PCI
>                         bridge to reassign resources.  PCI-PCI bridge
>                         can be specified, if resource windows need to
>                         be expanded.

Generally, it doesn't work as claimed from my observation.

 /* This quirk function enables us to force all memory resources which are
  * assigned to PCI devices, to be page-aligned.
  */
@@ -41,6 +54,42 @@
        int i;
        struct resource *r;
        resource_size_t old_start;
+
+       if (reassign_resources) {
+               if (dev->hdr_type == PCI_HEADER_TYPE_NORMAL &&
+                   (dev->class >> 8) == PCI_CLASS_BRIDGE_HOST) {
+                       /* PCI Host Bridge isn't a target device */
The comments is wrong -- we get invalid device type here, not the host 
bridge. Host bridge doesn't have a 'dev' at all.
+                       return;
+               }
+               if (is_reassigndev(dev)) {
+                       printk(KERN_INFO
+                               "PCI: Disable device and release resources"
+                               " [%s].\n", pci_name(dev));
+                       pci_disable_device(dev);
+
+                       for (i=0; i < PCI_NUM_RESOURCES; i++) {
+                               r = &dev->resource[i];
+                               if ((r == NULL) ||
How can 'r' be NULL?

+                                  !(r->flags & IORESOURCE_MEM))
+                                       continue;
+
+                               r->end = r->end - r->start;
+                               r->start = 0;
+
+                               if (i < PCI_BRIDGE_RESOURCES) {
+                                       pci_update_resource(dev, r, i);
+                               } else if (i == 8 || i == 9) {
The bridge even hasn't been scanned yet so this will *never* execute as 
expected.
+                                       /* need to update(clear) the Base/Limit
+                                        * register also, because PCI bridge is
+                                        * disabled and the resource is
+                                        * released.
+                                        */
+                                       pci_update_bridge(dev, i);
+                               }
+                       }
+               }
+               return;
+       }

        if (!pci_mem_align)
                return;
...

diff -r b54652ee29ef drivers/pci/setup-bus.c
--- a/drivers/pci/setup-bus.c   Thu Oct 02 11:29:02 2008 +0100
+++ b/drivers/pci/setup-bus.c   Wed Oct 08 12:12:27 2008 +0900
@@ -26,6 +26,7 @@
 #include <linux/cache.h>
 #include <linux/slab.h>

+#include "pci.h"

 #define DEBUG_CONFIG 1
 #if DEBUG_CONFIG
@@ -344,7 +345,8 @@

        list_for_each_entry(dev, &bus->devices, bus_list) {
                int i;
It most likely couldn't get here because the x86 PCI low level code has 
allocated resources for the bridge according to the BIOS and the 
function returns early.
-
+               int reassign = reassign_resources ? is_reassigndev(dev) : 0;
+
                for (i = 0; i < PCI_NUM_RESOURCES; i++) {
                        struct resource *r = &dev->resource[i];
                        unsigned long r_size;
@@ -352,6 +354,11 @@
                        if (r->parent || (r->flags & mask) != type)
                                continue;
                        r_size = r->end - r->start + 1;
+
+                       if (reassign) {
+                               r_size = ROUND_UP_TO_PAGESIZE(r_size);
+                       }
+
                        /* For bridges size != alignment */
                        align = (i < PCI_BRIDGE_RESOURCES) ? r_size : r->start;
                        order = __ffs(align) - 20;
diff -r b54652ee29ef drivers/pci/setup-res.c
--- a/drivers/pci/setup-res.c   Thu Oct 02 11:29:02 2008 +0100
+++ b/drivers/pci/setup-res.c   Wed Oct 08 12:12:27 2008 +0900
@@ -117,19 +117,96 @@
 }
 EXPORT_SYMBOL_GPL(pci_claim_resource);

+void
+pci_update_bridge(struct pci_dev *dev, int resno)
+{
+       struct resource *res = &dev->resource[resno];
+       struct pci_bus_region region;
+       u32 l, dw, base_up32, limit_up32;
+
+       if (dev->hdr_type != PCI_HEADER_TYPE_BRIDGE ||
+           (dev->class >> 8) != PCI_CLASS_BRIDGE_PCI) {
+               return;
+       }
+
+       if (!res->flags)
+               return;
+
+       switch (resno) {
+       case 8 :        /* MMIO Base/Limit */
+               pcibios_resource_to_bus(dev, &region, res);
+               if (res->flags & IORESOURCE_MEM &&
+                   !(res->flags & IORESOURCE_PREFETCH)) {
+                       l = (region.start >> 16) & 0xfff0;
+                       l |= region.end & 0xfff00000;
+               } else {
+                       l = 0x0000fff0;
+               }
+               pci_write_config_dword(dev, PCI_MEMORY_BASE, l);
+
+               break;
+
+       case 9 :        /* Prefetchable MMIO Base/Limit */
+               /* Clear out the upper 32 bits of PREF limit.
+                * If PCI_PREF_BASE_UPPER32 was non-zero, this temporarily
+                * disables PREF range, which is ok.
+                */
+               pci_write_config_dword(dev, PCI_PREF_LIMIT_UPPER32, 0);
+
+               /* Get PREF 32/64 bits Addressing mode */
+               pci_read_config_dword(dev, PCI_PREF_MEMORY_BASE, &dw);
+
+               pcibios_resource_to_bus(dev, &region, res);
+               if (res->flags & IORESOURCE_MEM &&
+                   res->flags & IORESOURCE_PREFETCH) {
+                       l = (region.start >> 16) & 0xfff0;
+                       l |= region.end & 0xfff00000;
+
+                       if (dw & PCI_PREF_RANGE_TYPE_64) {
+                               base_up32 = (region.start >> 32) & 0xffffffff;
+                               limit_up32 = (region.end >> 32) & 0xffffffff;
+                       } else {
+                               base_up32 = 0;
+                               limit_up32 = 0;
+                       }
+               } else {
+                       l = 0x0000fff0;
+                       base_up32 = 0xffffffff;
+                       limit_up32 = 0;
+               }
+               pci_write_config_dword(dev, PCI_PREF_MEMORY_BASE, l);
+               /* Set up the upper 32 bits of PREF base/limit. */
+               pci_write_config_dword(dev, PCI_PREF_BASE_UPPER32, base_up32);
+               pci_write_config_dword(dev, PCI_PREF_LIMIT_UPPER32, limit_up32);
+               break;
+       default :
+               BUG();
+               break;
+       }
+}
+
I don't see how this function 'expend' the resource window (even it was 
invoked). Actually it has many problems: 1, the dev->resource is not 
updated hence software doesn't know the new resource window size that 
can be granted to subordinate devices. 2, the values might be 
overwritten later by pci_setup_bridge. 3, the registers are changed 
without checking if resource range are available, which means it may 
conflict with other bridge.
 int pci_assign_resource(struct pci_dev *dev, int resno)
 {
        struct pci_bus *bus = dev->bus;
        struct resource *res = dev->resource + resno;
        resource_size_t size, min, align;
        int ret;
+       int reassigndev = reassign_resources ? is_reassigndev(dev) : 0;

        size = res->end - res->start + 1;
        min = (res->flags & IORESOURCE_IO) ? PCIBIOS_MIN_IO : PCIBIOS_MIN_MEM;
        /* The bridge resources are special, as their
           size != alignment. Sizing routines return
           required alignment in the "start" field. */
-       align = (resno < PCI_BRIDGE_RESOURCES) ? size : res->start;
+       if (resno < PCI_BRIDGE_RESOURCES) {
+               align = size;
+               if ((reassigndev) &&
+                   (res->flags & IORESOURCE_MEM)) {
+                       align = ROUND_UP_TO_PAGESIZE(align);
Different MMIO resrouces of a device may require different alignments. 
Using page size for all may cause problem. The alignment should be a 
bigger value between page size and resource size. Or the best way is 
keep aligned resources of a device and just reassign the unaligned 
resources in the quirk_align_mem_resources.
+               }
+       } else {
+               align = res->start;
+       }

        /* First, try exact prefetching match.. */
        ret = pci_bus_alloc_resource(bus, res, size, align, min,
@@ -154,6 +231,9 @@
                        resno, (unsigned long long)size,
                        (unsigned long long)res->start, pci_name(dev));
        } else if (resno < PCI_BRIDGE_RESOURCES) {
+               printk(KERN_DEBUG "PCI: Assign resource(%d) on %s "
+                       "%016llx - %016llx\n", resno, pci_name(dev),
+                       (u64)res->start, (u64)res->end);
                pci_update_resource(dev, res, resno);
        }




_______________________________________________
Xen-devel mailing list
Xen-devel@xxxxxxxxxxxxxxxxxxx
http://lists.xensource.com/xen-devel

_______________________________________________
Xen-devel mailing list
Xen-devel@xxxxxxxxxxxxxxxxxxx
http://lists.xensource.com/xen-devel