Re: [Xen-ia64-devel] [PATCH] Add memory operations for xen/ia64

[Alex Williamson <alex.williamson@xxxxxx>]

Hi Kevin,

   A few minor comments below.  Thanks,

        Alex

On Mon, 2006-04-03 at 21:56 +0800, Tian, Kevin wrote:
> --- a/xen/arch/ia64/xen/dom0_ops.c      Thu Mar 30 04:19:28 2006
> +++ b/xen/arch/ia64/xen/dom0_ops.c      Mon Apr  3 20:57:58 2006
> @@ -157,40 +157,45 @@
>       */
>      case DOM0_GETMEMLIST:
>      {
> -        unsigned long i;
>          struct domain *d = find_domain_by_id(op->u.getmemlist.domain);
>          unsigned long start_page = op->u.getmemlist.max_pfns >> 32;
>          unsigned long nr_pages = op->u.getmemlist.max_pfns & 0xffffffff;
>          unsigned long mfn;
> +        struct list_head *list_ent;
> +        int i = 0;

Looks like i should probably still be an unsigned long here (even though
overflow is unlikely).

> --- a/xen/arch/ia64/xen/domain.c        Thu Mar 30 04:19:28 2006
> +++ b/xen/arch/ia64/xen/domain.c        Mon Apr  3 20:57:58 2006
> +void build_physmap_table(struct domain *d)
> +{
> +       struct list_head *list_ent = d->page_list.next;
> +       int i = 0;
> +       unsigned long mfn;
> +
> +       ASSERT(!d->arch.physmap_built);
> +       while(list_ent != &d->page_list) {
> +           mfn = page_to_mfn(list_entry(
> +               list_ent, struct page_info, list));
> +           assign_domain_page(d, i << PAGE_SHIFT, mfn << PAGE_SHIFT);
> +

Same here?  unsigned long i?

> @@ -664,12 +648,13 @@
>                         }
>                 }
>         }
> -       /* if lookup fails and mpaddr is "legal", "create" the page */
>         if ((mpaddr >> PAGE_SHIFT) < d->max_pages) {
> -               if (assign_new_domain_page(d,mpaddr)) goto tryagain;
> -       }
> -       printk("lookup_domain_mpa: bad mpa 0x%lx (> 0x%lx)\n",
> -               mpaddr, (unsigned long) d->max_pages<<PAGE_SHIFT);
> +               //if (assign_new_domain_page(d,mpaddr)) goto tryagain;
                                                          ^^^^^^^^^^^^^
This causes a warning about tryagain being defined but unused.  Should
we just remove this commented out line and the label?

> --- a/xen/arch/ia64/vmx/vmx_init.c      Thu Mar 30 04:19:28 2006
> +++ b/xen/arch/ia64/vmx/vmx_init.c      Mon Apr  3 20:57:58 2006
> @@ -327,13 +327,16 @@
>  #define VMX_SYS_PAGES  (2 + (GFW_SIZE >> PAGE_SHIFT))
>  #define VMX_CONFIG_PAGES(d) ((d)->max_pages - VMX_SYS_PAGES)
>  
> -int vmx_alloc_contig_pages(struct domain *d)
> -{
> -       unsigned long i, j, start,tmp, end, pgnr, conf_nr;
> +int vmx_build_physmap_table(struct domain *d)
> +{
> +       unsigned long i, j, start, tmp, end, mfn;
>         struct page_info *page;

page appears to be unused here now, causes a warning.

-- 
Alex Williamson                             HP Linux & Open Source Lab


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