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

["Jan Beulich" <JBeulich@xxxxxxxxxx>]

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