29 messages in com.xensource.lists.xen-ia64-devel[Xen-ia64-devel] Re: [4/4] Issue iore...
FromSent OnAttachments
Jun Kamada17 May 2007 21:43.patch, .patch, .patch, 1 more
Akio Takebe25 May 2007 20:26 
Jun Kamada29 May 2007 22:23 
Jun Kamada29 May 2007 22:38.patch
Jun Kamada29 May 2007 22:40.patch
Jun Kamada29 May 2007 22:43.patch
Jun Kamada29 May 2007 22:45.patch
Alex Williamson30 May 2007 22:40 
Jun Kamada31 May 2007 18:24 
Isaku Yamahata01 Jun 2007 17:57 
Jun Kamada04 Jun 2007 03:18 
Isaku Yamahata04 Jun 2007 05:07 
Jun Kamada04 Jun 2007 21:28 
Isaku Yamahata04 Jun 2007 22:46 
Jun Kamada04 Jun 2007 23:15 
Jun Kamada12 Jun 2007 23:51 
Jun Kamada12 Jun 2007 23:53.patch
Jun Kamada12 Jun 2007 23:54.patch
Jun Kamada12 Jun 2007 23:54.patch
Jun Kamada12 Jun 2007 23:55.patch
Alex Williamson14 Jun 2007 13:24 
Jun Kamada14 Jun 2007 22:02 
Jun Kamada13 Jul 2007 02:36 
Jun Kamada13 Jul 2007 02:38.patch
Jun Kamada13 Jul 2007 02:39.patch
Jun Kamada13 Jul 2007 02:40.patch
Alex Williamson19 Jul 2007 12:04 
Jun Kamada20 Jul 2007 04:18.patch
Alex Williamson30 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;