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
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
|