On Wed, 2006年05月10日 at 09:51 -0500, Hollis Blanchard wrote:
> On Wed, 2006年05月10日 at 08:06 +0200, Simon Kagstrom wrote:
> > 
> > I won't argue for an incorrect fix, but as the code is right now it
> > segmentation faults because the virtual address passed to
> > 
> > page = page_array[va >> PAGE_SHIFT] << PAGE_SHIFT;
> > 
> > (with libxc incorrectly believing paging is disabled) accesses outside
> > of page_array. I'll keep the patch privately for now since gdbserver
> > breaks without it. 
>
> Yes, and the reason is that page_array is supposed to be indexed with
> *physical* addresses. The current code thinks that paging is disabled
> (because CR0 is bad), assumes a virtual address is physical, and tries
> to index into the array with it.
>
> Pretty much every use of page_array needs to be abstracted so that it
> does the right thing on both HVM and normal guests (normal guests will
> have machine addresses in its page tables; HVM guests will have
> physical). It's very unfortunate that the people who worked on this
> code seem not to have tested or even thought about paravirtualized
> guests.
To elaborate on my previous mail, it's not just CR0/paging at fault. For
example, this use of page_array:
 ...
 l3p = l4[l4_table_offset(va)] >> PAGE_SHIFT;
 l3p = page_array[l3p];
 ...
in map_domain_va_64() is obviously incorrect for paravirtualized
domains.
Also, I noticed there's another place that already tests VGCF_HVM_GUEST
before paging_enabled(), which I guess is where you got the idea for
your patch.
Simon, would you care to submit the more complete patch?
-- 
Hollis Blanchard
IBM Linux Technology Center
_______________________________________________
Xen-devel mailing list
Xen-devel@xxxxxxxxxxxxxxxxxxx
http://lists.xensource.com/xen-devel