Re: [PATCH] Support swap a page from user space tools -- Was RE: [RFC][PATCH] Basic support for page offline

[Tim Deegan <Tim.Deegan@citrix.com>]

Hi, 

At 10:24 +0000 on 18 Mar (1237371884), Jiang, Yunhong wrote:
> Tim, this is the implementation as discussed.
> The swap_page.patch is for HV side change, basically it call the mod_lx_entry to update the table.

That seems good.  One or two nits with that patch:
 
 - You're passing a physical address (of the PTE to update) in an MFN
   field.  That's not going to be big enough on all platforms.  Also
   it's pretty confusing.

 - The "swap" operation takes the physical address of a PTE and a new
   MFN.  Why not a new PTE?  And if it's going to be called "swap", why
   not take the old PTE too and do an atomic update?  (or just call it
   something else and only take the new PTE?)

 - Why the checks for the validity of the old MFN before the change?
   What are you guarding against?

And please document the hypercall, specially since its side-effects
could be quite surprising. 

> The free_page.patch is the function from user space tools to offlien a page.

This is much better than the previous version, thanks.

Cheers,

Tim.

> 
> Thanks
> Yunhong Jiang
> 
> Jiang, Yunhong <> wrote:
> > Tim Deegan <mailto:Tim.Deegan@citrix.com> wrote:
> >> At 14:37 +0000 on 19 Feb (1235054276), Jiang, Yunhong wrote:
> >>> 
> >>>>> It should be possible to have the tools do all the PTE manipulations
> >>>>> with MMU update hypercalls (I think -- Keir may correct me here). Then
> >>>>> the final hypercall to surrender the page will fail if the grant tables
> >>>>> are wrong; if it does, put the PTEs back and fall back to a live
> >>>>> migration.  Isn't that what your in-xen code does anyway?
> >>> 
> >>> Tim, after checking the code more carefully, seems currently
> >> the MMU update hypercalls (including mod_lx_entry ) assume it
> >> is for current domain, while in our usage model, it will
> >> update MMU for other domain, so I will try to do following
> >> changes: 1) change mod_lx_entry() to get a domain parameter 2)
> >> Add a new hypercall (or a new command to do_mmu_update ) to
> >> update the MMU for other domain. I'm not sure if there are
> >> other usage model for such requirement, and if such changes
> >> acceptable? Any feedback is welcome.
> >>> 
> >> 
> >> Sorry for the delay -- I was travelling around the summit and this got
> >> lost.  Yes, I think this is an OK approach.  I doubt there will be many
> >> other users of such a hypercall since most OSes will get upset by their
> >> PTEs changing under their feet, but I prefer it to the current patch.
> > 
> > Sure, I will do this way.
> > 
> > Thanks
> > Yunhong Jiang
> > 
> >> 
> >> Cheers,
> >> 
> >> Tim.
> >> 
> >> --
> >> Tim Deegan <Tim.Deegan@citrix.com>
> >> Principal Software Engineer, Citrix Systems (R&D) Ltd.
> >> [Company #02300071, SL9 0DZ, UK.]



-- 
Tim Deegan <Tim.Deegan@citrix.com>
Principal Software Engineer, Citrix Systems (R&D) Ltd.
[Company #02300071, SL9 0DZ, UK.]