29 messages in com.xensource.lists.xen-ia64-develRe: [Xen-ia64-devel] [Patch 4/4] Xwin...| From | Sent On | Attachments |
|---|---|---|
| Jun Kamada | 17 May 2007 21:43 | .patch, .patch, .patch, 1 more |
| Akio Takebe | 25 May 2007 20:26 | |
| Jun Kamada | 29 May 2007 22:23 | |
| Jun Kamada | 29 May 2007 22:38 | .patch |
| Jun Kamada | 29 May 2007 22:40 | .patch |
| Jun Kamada | 29 May 2007 22:43 | .patch |
| Jun Kamada | 29 May 2007 22:45 | .patch |
| Alex Williamson | 30 May 2007 22:40 | |
| Jun Kamada | 31 May 2007 18:24 | |
| Isaku Yamahata | 01 Jun 2007 17:57 | |
| Jun Kamada | 04 Jun 2007 03:18 | |
| Isaku Yamahata | 04 Jun 2007 05:07 | |
| Jun Kamada | 04 Jun 2007 21:28 | |
| Isaku Yamahata | 04 Jun 2007 22:46 | |
| Jun Kamada | 04 Jun 2007 23:15 | |
| Jun Kamada | 12 Jun 2007 23:51 | |
| Jun Kamada | 12 Jun 2007 23:53 | .patch |
| Jun Kamada | 12 Jun 2007 23:54 | .patch |
| Jun Kamada | 12 Jun 2007 23:54 | .patch |
| Jun Kamada | 12 Jun 2007 23:55 | .patch |
| Alex Williamson | 14 Jun 2007 13:24 | |
| Jun Kamada | 14 Jun 2007 22:02 | |
| Jun Kamada | 13 Jul 2007 02:36 | |
| Jun Kamada | 13 Jul 2007 02:38 | .patch |
| Jun Kamada | 13 Jul 2007 02:39 | .patch |
| Jun Kamada | 13 Jul 2007 02:40 | .patch |
| Alex Williamson | 19 Jul 2007 12:04 | |
| Jun Kamada | 20 Jul 2007 04:18 | .patch |
| Alex Williamson | 30 Jul 2007 15:04 |
| Subject: | Re: [Xen-ia64-devel] [Patch 4/4] Xwindow: Modify pci_acpi_scan_root()![]() |
|---|---|
| From: | Alex Williamson (alex...@hp.com) |
| Date: | 06/14/2007 01:24:02 PM |
| List: | com.xensource.lists.xen-ia64-devel |
Hi,
Some more comments below. Thanks for switching to the standard list implementation, it looks better. How do you propose coordinating the patches for xen-ia64-devel vs the one for xen-devel? Should we get the latter in first, then proceed w/ the xen-ia64-devel patch? Thanks,
Alex
On Wed, 2007-06-13 at 15:56 +0900, Jun Kamada wrote:
# HG changeset patch # User Jun Kamada <ka...@jp.fujitsu.com> # Date 1181706151 -32400 # Node ID 76e7ecd69d2ce751bf33784f62762c2ba8ed7162 # Parent 7286707307032b6b3117a4f48aa4f0fdce6e7587 Issue ioremap hypercall in pci_acpi_scan_root() in order to map /dev/mem.
Signed-off-by: Jun Kamada <ka...@jp.fujitsu.com>
diff -r 728670730703 -r 76e7ecd69d2c linux-2.6-xen-sparse/arch/ia64/pci/pci.c --- a/linux-2.6-xen-sparse/arch/ia64/pci/pci.c Wed Jun 13 12:38:18 2007 +0900 +++ b/linux-2.6-xen-sparse/arch/ia64/pci/pci.c Wed Jun 13 12:42:31 2007 +0900 @@ -29,6 +29,15 @@ #include <asm/smp.h> #include <asm/irq.h> #include <asm/hw_irq.h> + +#ifdef CONFIG_XEN +struct ioremap_issue_list { + struct list_head listp; + unsigned long start; + unsigned long end; +}; +typedef struct ioremap_issue_list ioremap_issue_list_t; +#endif /* CONFIG_XEN */
/* * Low-level SAL-based PCI configuration access functions. Note that SAL @@ -332,6 +341,165 @@ pcibios_setup_root_windows(struct pci_bu } }
+#ifdef CONFIG_XEN + +#define MIN(x,y) (((x) <= (y)) ? (x) : (y)) +#define MAX(x,y) (((x) >= (y)) ? (x) : (y))
min() and max() are already defined in kernel.h, can you make use of those?
+static int +__add_issue_list(unsigned long start, unsigned long end, + ioremap_issue_list_t *top) +{ + ioremap_issue_list_t *ptr, *new; + + if (start > end) { + printk(KERN_ERR "%s: Internal error (start addr > end addr)\n", + __FUNCTION__); + return 0; + } + + start &= ~(PAGE_SIZE - 1);
Use PAGE_MASK
+ end |= (PAGE_SIZE - 1);
And ~PAGE_MASK
+ + list_for_each_entry(ptr, &(top->listp), listp) { + if (((ptr->start >= start) && (ptr->start <= end + 1)) || + ((ptr->end >= start - 1) && (ptr->end <= end)) || + ((start >= ptr->start) && (start <= ptr->end) && + (end >= ptr->start) && (end <= ptr->end))) { + ptr->start = MIN(start, ptr->start); + ptr->end = MAX(end, ptr->end); + return 0; + }
Can't this be greatly simplified? Instead of checking all the degrees of overlap, just look for the non-overlap cases. Everything else is overlap
if (ptr->start > end + 1 || ptr->end + 1 < start) continue;
ptr->start = min(start, ptr->start); ptr->end = max(end, ptr->end); return 0;
However, what if this grows the range such that it bumps into the entries around it. Do you plan to merge them? Maybe keeping the list sorted would make this easier.
+ } + + new = kmalloc(sizeof(ioremap_issue_list_t), GFP_KERNEL); + if (new == NULL) { + printk(KERN_ERR "%s: Could not allocate memory. " + "HYPERVISOR_ioremap will not be issued\n", + __FUNCTION__); + return -ENOMEM; + } + + new->start = start; + new->end = end; + + list_add(&(new->listp), &(top->listp)); + + return 0; +} + +static int +__make_issue_list(struct resource *ptr, ioremap_issue_list_t *top) { + unsigned int curr_lvl = 1; + unsigned int prev_lvl = 0; + int ret; + + for ( ;; ) { + if (ptr->flags & IORESOURCE_MEM) { + ret = __add_issue_list(ptr->start, ptr->end, top); + if (ret) + return ret; + } +
Thanks, the comments below are helpful, but wouldn't it make sense to do this as a recursive algorithm? Doesn't something like this do the same thing (untested)?
int scan_foo(struct resource *res, ioremap_issue_list_t *list) { int ret;
if (res->child) { ret = scan_foo(res->child, list); if (ret) return ret; } if (res->sibling) { ret = scan_foo(res->sibling, list); if (ret) return ret; }
if (ptr->flags & IORESOURCE_MEM) { ret = __add_issue_list(res->start, res->end, list); if (ret) return ret; } return 0; }
+ /* means that the "ptr" came here from parent (upper level) */ + if (curr_lvl > prev_lvl) { + /* child exists, so go down to the child */ + if (ptr->child != 0) { + ptr = ptr->child; + curr_lvl++; + prev_lvl++; + /* sibling exists, so go to the sibling */ + } else if (ptr->sibling != 0) { + ptr = ptr->sibling; + /* no child and no sibling exist, so go up to parent */ + } else { + ptr = ptr->parent; + curr_lvl--; + prev_lvl++; + if (curr_lvl == 0) + return 0; + } + + /* means that the "ptr" came here from child (lower level) */ + } else if (curr_lvl < prev_lvl) { + /* sibling exists, so go to the sibling */ + if (ptr->sibling != 0) { + ptr = ptr->sibling; + prev_lvl = curr_lvl - 1; + /* no sibling exists, so go up to parent */ + } else { + ptr = ptr->parent; + curr_lvl--; + prev_lvl--; + if (curr_lvl == 0) + return 0; + } + } else { + printk(KERN_ERR "%s: Internal error. " + "Must not reach here\n", __FUNCTION__); + return -EFAULT; + } + } + + return 0; +} + +static int +__issue_ioremap(struct ioremap_issue_list *top) +{ + ioremap_issue_list_t *ptr, *tmp_ptr; + unsigned int offset; + + list_for_each_entry(ptr, &(top->listp), listp) { + offset = HYPERVISOR_ioremap(ptr->start, + ptr->end - ptr->start + 1); + if (offset == ~0) { + printk(KERN_ERR "%s: HYPERVISOR_ioremap() failed. " + "Address Range: 0x%016lx-0x%016lx\n", + __FUNCTION__, ptr->start, ptr->end); + } + } + + list_for_each_entry_safe(ptr, tmp_ptr, &(top->listp), listp) { + list_del(&(ptr->listp)); + kfree(ptr); + }
These could be done in the same pass through the list.
+ return 0; +} + +static int +do_ioremap_on_resource_list(struct resource *ptr) +{ + ioremap_issue_list_t *ioremap_issue_list_top; + int ret; + + ioremap_issue_list_top = kmalloc(sizeof(ioremap_issue_list_t), + GFP_KERNEL); + if (ioremap_issue_list_top == NULL) + return -ENOMEM;
The top of the list doesn't need to be an ioremap_issue_list_t, it can just be a struct list. That could just be put on the stack rather than dynamically allocated.
+ + INIT_LIST_HEAD(&(ioremap_issue_list_top->listp)); +
If you took the recursive approach above, these could be eliminated, just call scan_foo(ptr, &list).
+ if (ptr->child != 0) { + ret = __make_issue_list(ptr->child, ioremap_issue_list_top); + if (ret) {
memory leak - be careful to do list_del & kfree should anything fail
+ return ret; + } + } + if (ptr->sibling != 0) { + ret = __make_issue_list(ptr->sibling, ioremap_issue_list_top); + if (ret) {
memory leak - ditto
+ return ret; + } + } + + (void)__issue_ioremap(ioremap_issue_list_top); + + return 0; +} +#endif /* CONFIG_XEN */ + struct pci_bus * __devinit pci_acpi_scan_root(struct acpi_device *device, int domain, int bus) { @@ -374,6 +542,18 @@ pci_acpi_scan_root(struct acpi_device *d pbus = pci_scan_bus_parented(NULL, bus, &pci_root_ops, controller); if (pbus) pcibios_setup_root_windows(pbus, controller); + +#ifdef CONFIG_XEN + if (is_initial_xendomain()) { + if (do_ioremap_on_resource_list(&iomem_resource) != 0) { + printk(KERN_ERR + "%s: Counld not issue HYPERVISOR_ioremap " + "due to lack of memory or hypercall failure\n", + __FUNCTION__); + goto out3; + } + } +#endif /* CONFIG_XEN */
return pbus;
-- Alex Williamson HP Open Source & Linux Org.
_______________________________________________ Xen-ia64-devel mailing list Xen-...@lists.xensource.com http://lists.xensource.com/xen-ia64-devel





.patch, .patch, .patch, 1 more