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@us.ibm.com
_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xensource.com
http://lists.xensource.com/xen-devel
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@us.ibm.com> 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@us.ibm.com
_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xensource.com
http://lists.xensource.com/xen-devel
On Wed, Jun 22, 2005 at 01:14:34AM -0600, Jan Beulich wrote:> 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).Good point sRp -- Scott Parish Signed-off-by: srparish@us.ibm.com _______________________________________________ Xen-devel mailing list Xen-devel@lists.xensource.com http://lists.xensource.com/xen-devel