29 messages in com.xensource.lists.xen-ia64-devel[Xen-ia64-devel] Re: [4/4] Issue iore...| 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: | [Xen-ia64-devel] Re: [4/4] Issue ioremap hypercall in pci_acpi_scan_root() in order to map /dev/mem![]() |
|---|---|
| From: | Jun Kamada (ka...@jp.fujitsu.com) |
| Date: | 07/20/2007 04:18:28 AM |
| List: | com.xensource.lists.xen-ia64-devel |
| Attachments: |
Hi, Alex-san,
Thank you for your helpful comments. I will attach a modified patch.
Thanks
On Thu, 19 Jul 2007 13:04:44 -0600 Alex Williamson <alex...@hp.com> wrote:
While we're waiting on staging to get pushed out, here are a few more comments. Casting ioremap_issue_list_top as a struct ioremap_issue_list is awkward, and I think unnecessary. The gotos are easy to remove too. You might also want to consider a few simple variable renames, for instance "top" is used as both a struct resource and a struct ioremap_issue_list. A few additional cleanups below. Thanks,
Alex
On Fri, 2007-07-13 at 18:40 +0900, Jun Kamada wrote:
# HG changeset patch # User Jun Kamada <ka...@jp.fujitsu.com> # Date 1184348594 -32400 # Node ID b82a7428f53a5d03c964fd30d21100429376e64f # Parent 031ea456e2756b3f7c10c00f7fcdccb4fc01baa2 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 031ea456e275 -r b82a7428f53a arch/ia64/pci/pci.c --- a/arch/ia64/pci/pci.c Sat Jul 14 02:41:26 2007 +0900 +++ b/arch/ia64/pci/pci.c Sat Jul 14 02:43:14 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 @@ -337,6 +346,182 @@ pcibios_setup_root_windows(struct pci_bu } }
+#ifdef CONFIG_XEN +static void +__cleanup_issue_list(ioremap_issue_list_t *top)
s/ioremap_issue_list_t/struct list/
+{ + ioremap_issue_list_t *ptr, *tmp_ptr; + + list_for_each_entry_safe(ptr, tmp_ptr, &(top->listp), listp) {
s/&(top->listp)/top/
+ list_del(&(ptr->listp)); + kfree(ptr); + } +} + +static int +__add_issue_list(unsigned long start, unsigned long end, + ioremap_issue_list_t *top)
s/ioremap_issue_list_t/struct list/
+{ + ioremap_issue_list_t *ptr, *new; + + if (start > end) { + printk(KERN_ERR "%s: Internal error (start addr > end addr)\n", + __FUNCTION__); + return 0; + } + + /* Head of the resource structure list contains */ + /* dummy val.(start=0, end=~0), so skip it */ + if ((start == 0) && (end == ~0)) {
+ return 0; + } + + start &= PAGE_MASK; + end |= ~PAGE_MASK; + + /* We can merge specified address range into existing entry */ + list_for_each_entry(ptr, &(top->listp), listp) {
s/&(top->listp)/top
+ if ((ptr->start > end + 1) || (ptr->end + 1 < start)) + continue; + ptr->start = min(start, ptr->start); + ptr->end = max(end, ptr->end); + goto out; + } + + /* We could not merge, so create new entry */ + 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; + + /* Insert the new entry to the list by ascending order */ + if (list_empty(&(top->listp))) {
s/&(top->listp)/top
+ list_add_tail(&(new->listp), &(top->listp));
s/&(top->listp)/top
+ goto out;
s/goto out/return 0/
+ } + list_for_each_entry(ptr, &(top->listp), listp) {
s/&(top->listp)/top
+ if (new->start > ptr->start) + continue; + list_add(&(new->listp), ((struct list_head *)ptr)->prev); + goto out;
s/goto out/return 0/
+ } + list_add_tail(&(new->listp), &(top->listp));
s/&(top->listp)/top
+ +out:
s/out://
+ return 0; +} + +static int +__make_issue_list(struct resource *ptr, ioremap_issue_list_t *top)
s/ioremap_issue_list_t/struct list/
+{ + int ret; + + if (ptr->child) { + ret = __make_issue_list(ptr->child, top); + if (ret) + return ret; + } + if (ptr->sibling) { + ret = __make_issue_list(ptr->sibling, top); + if (ret) + return ret; + } + + if (ptr->flags & IORESOURCE_MEM) { + ret = __add_issue_list(ptr->start, ptr->end, top); + if (ret) + return ret; + } + + return 0; +} + +static void +__compress_issue_list(ioremap_issue_list_t *top) +{ + ioremap_issue_list_t *ptr, *tmp_ptr, *next; + int compressed; + + /* merge adjacent entries, if overlapped */ + /* (assumes entries are sorted by ascending order) */
s/assumes// - At least I hope we're sure they're sorted ;)
+ do { + compressed = 0; + + list_for_each_entry_safe(ptr, tmp_ptr, &(top->listp), listp) {
s/&(top->listp)/top
+ if (list_is_last((struct list_head *)ptr, + &(top->listp)) == 0) {
s/&(top->listp)/top
Rather than adding another level of nesting here, how about:
if (list_is_last((struct list_head *)ptr, top)) continue; /* or maybe break */
+ next = (ioremap_issue_list_t *) + (((struct list_head *)ptr)->next); + if (next->start <= (ptr->end) + 1) { + next->start = min(ptr->start, + next->start); + next->end = max(ptr->end, + next->end); + + list_del(&(ptr->listp)); + kfree(ptr); + compressed = 1; + } + } + } + } while (compressed == 1);
Does this really need the do/while? We're expanding the next list entry and removing the current, so it seems like we complete the merging in one pass.
+} + +static int +__issue_ioremap(ioremap_issue_list_t *top)
s/ioremap_issue_list_t/struct list/
+{ + ioremap_issue_list_t *ptr, *tmp_ptr; + unsigned int offset; + + list_for_each_entry_safe(ptr, tmp_ptr, &(top->listp), listp) {
s/&(top->listp)/top
+ 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_del(&(ptr->listp)); + kfree(ptr); + } + + return 0; +} + +static int +do_ioremap_on_resource_list(struct resource *top) +{ + struct list_head ioremap_issue_list_top = { + &ioremap_issue_list_top, + &ioremap_issue_list_top + };
Use:
LIST_HEAD(ioremap_issue_list_top);
+ int ret; + + ret = __make_issue_list(top, + (ioremap_issue_list_t *)(&ioremap_issue_list_top));
s/(ioremap_issue_list_t *)//
+ if (ret) { + __cleanup_issue_list((ioremap_issue_list_t *)
s/(ioremap_issue_list_t *)//
+ (&ioremap_issue_list_top)); + return ret; + } + + __compress_issue_list((ioremap_issue_list_t *)
s/(ioremap_issue_list_t *)//
+ (&ioremap_issue_list_top)); + + (void)__issue_ioremap((ioremap_issue_list_t *)
s/(ioremap_issue_list_t *)//
+ (&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) { @@ -379,6 +564,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.
Jun Kamada Linux Technology Development Div. Server Systems Unit Fujitsu Ltd. ka...@jp.fujitsu.com





.patch, .patch, .patch, 1 more