RE: [Xen-devel] [PATCH][VT] Patch to allow VMX domainsto be destroyedor shut down cleanly

["Jiang, Yunhong" <yunhong.jiang@xxxxxxxxx>]

Keir Fraser wrote:
> On 19 Sep 2005, at 06:54, Jiang, Yunhong wrote:
> 
>> More exactly should be:
>>      if (!vmx_paging_enabled(d)) {
>>                      get_page for new mfn
>>                      put_page for old mfn
>>      }
> 
> What would old_mfn be in this case?
> 
> If we do any cr3 ref-counting on cr0 writes I would expect it to be:
>   * put_page() on clearing of CR0_PG
>   * get_page() on setting of CR0_PG
> 
> Given that writing cr3 currently does a get_page/put_page regardless
> of the setting of CR0_PG, is there any need to be doing get_page()
> within vmx_set_cr0()?

I'm not sure if the get_page() on vmx_set_cr0 has purpose of sanity check , 
otherwise it can be removed. In fact, I think the put_page has the effect to 
remove it.

> 
> I think it's more correct to do get/put in vmx_set_cr0 (at that point
> we can know what format the pagetables should be, for example
> (32/32p/64). But I think it's a bigger cleanup patch.

Do you mean put all get/put on vmx_set_cr0 ? even for paging enabled case?

And there is one more issue .If the guest is destroyed before the kernel 
enabling paging, how about the page pointed by cr3? It may cause one referece 
count left. The reason is, currently when destroy domain, only 
domain->guest_table is released. However, if the paging is not enabled, then 
the get_page on vmx_set_cr0 has no corresponding put_page, which may cause 
problem. Hope comments on this point.

> 
> Whatever you go for: please send a complete tested patch. :-)

Sure！

> 
>   -- Keir


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