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

["Tian, Kevin" <kevin.tian@xxxxxxxxx>]

Hi, Alex,
        Thanks for comments, and here comes updated one. Please apply.

- Kevin

>-----Original Message-----
>From: Alex Williamson [mailto:alex.williamson@xxxxxx]
>Sent: 2006年4月4日 4:18
>To: Tian, Kevin
>Cc: xen-ia64-devel@xxxxxxxxxxxxxxxxxxx
>Subject: Re: [Xen-ia64-devel] [PATCH] Add memory operations for
>xen/ia64
>
>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

Attachment: add_memory_op.patch
Description: add_memory_op.patch

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