WARNING - OLD ARCHIVES

This is an archived copy of the Xen.org mailing list, which we have preserved to ensure that existing links to archives are not broken. The live archive, which contains the latest emails, can be found at http://lists.xen.org/
   
 
 
Xen 
 
Home Products Support Community News
 
   
 

xen-devel

Re: [Xen-devel] [patch] unwanted sign extending

To: <xen-devel@xxxxxxxxxxxxxxxxxxx>, "Scott Parish" <srparish@xxxxxxxxxx>
Subject: Re: [Xen-devel] [patch] unwanted sign extending
From: "Jan Beulich" <JBeulich@xxxxxxxxxx>
Date: Wed, 22 Jun 2005 01:14:34 -0600
Delivery-date: Wed, 22 Jun 2005 07:13:30 +0000
Envelope-to: www-data@xxxxxxxxxxxxxxxxxxx
In-reply-to: <20050621201030.GB16276@xxxxxxxxxx>
List-help: <mailto:xen-devel-request@lists.xensource.com?subject=help>
List-id: Xen developer discussion <xen-devel.lists.xensource.com>
List-post: <mailto:xen-devel@lists.xensource.com>
List-subscribe: <http://lists.xensource.com/cgi-bin/mailman/listinfo/xen-devel>, <mailto:xen-devel-request@lists.xensource.com?subject=subscribe>
List-unsubscribe: <http://lists.xensource.com/cgi-bin/mailman/listinfo/xen-devel>, <mailto:xen-devel-request@lists.xensource.com?subject=unsubscribe>
References: <20050621201030.GB16276@xxxxxxxxxx>
Sender: xen-devel-bounces@xxxxxxxxxxxxxxxxxxx
I'd think that for correctness this should also be done to alloc_l2_table. And 
I also think that this is still wrong for 64 bits: Shifting left an unsigned 
yields an unsigned, and since 'i' can range from 0 to 511 and the shift count 
is 30, the result is going to be truncated. That is, the code should be

        vaddr = (unsigned long)i << L3_PAGETABLE_SHIFT; 

(and again, for consistency it should also be done so in alloc_l2_table).

Jan

>>> "Scott Parish" <srparish@xxxxxxxxxx> 21.06.05 22:10:30 >>>

static int alloc_l3_table(struct pfn_info *page) 
{ 
  ...
    unsigned long  vaddr; 
    unsigned int   i; 
  ...
    for ( i = 0; i < L3_PAGETABLE_ENTRIES; i++ ) 
    { 
        vaddr = i << L3_PAGETABLE_SHIFT; 
  ...
    } 
...
}

"i" gets sign extended when its shifted, so vaddr has all its high
bits set. Because of that some l2 page_type's come out looking like
PGT_writable instead of PGT_l2. Eventually this leads to an attempt to
call put_page_type on the page twice, once when cleaning up recursively
from l4, and once from walking the raw frames list. The second
put_page_type hits the ASSERT that the type count isn't 0.

With the attached patch, i can completely run a simple "hello world"
domu, and its cleanup. Linux domu still probably doesn't work.

sRp

-- 
Scott Parish
Signed-off-by: srparish@xxxxxxxxxx


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

<Prev in Thread] Current Thread [Next in Thread>