Re: [PATCH v8 03/15] x86/mm: rewrite virt_to_xen_l*e

[Julien Grall <julien@xxxxxxx>]

Hi,

On 13/08/2020 17:08, Hongyan Xia wrote:
> On Fri, 2020-08-07 at 16:05 +0200, Jan Beulich wrote:
> > On 27.07.2020 16:21, Hongyan Xia wrote:
> > > From: Wei Liu <wei.liu2@xxxxxxxxxx>
> > >
> > > Rewrite those functions to use the new APIs. Modify its callers to
> > > unmap
> > > the pointer returned. Since alloc_xen_pagetable_new() is almost
> > > never
> > > useful unless accompanied by page clearing and a mapping, introduce
> > > a
> > > helper alloc_map_clear_xen_pt() for this sequence.
> > >
> > > Note that the change of virt_to_xen_l1e() also requires
> > > vmap_to_mfn() to
> > > unmap the page, which requires domain_page.h header in vmap.
> > >
> > > Signed-off-by: Wei Liu <wei.liu2@xxxxxxxxxx>
> > > Signed-off-by: Hongyan Xia <hongyxia@xxxxxxxxxx>
> > > Reviewed-by: Jan Beulich <jbeulich@xxxxxxxx>
> > >
> > > ---
> > > Changed in v8:
> > > - s/virtual address/linear address/.
> > > - BUG_ON() on NULL return in vmap_to_mfn().
> > The justification for this should be recorded in the description. In
> Will do.
>
> > reply to v7 I did even suggest how to easily address the issue you
> > did notice with large pages, as well as alternative behavior for
> > vmap_to_mfn().
> One thing about adding SMALL_PAGES is that vmap is common code and I am
> not sure if the Arm side is happy with it.
At the moment, Arm is only using small mapping but I plan to change that 
soon because we have regions that can be fairly big.
Regardless that, the issue with vmap_to_mfn() is rather x86 specific. So 
I don't particularly like the idea to expose such trick in common code.
Even on x86, I think this is not the right approach. Such band-aid will 
impact the performance as, assuming superpages are used, it will take 
longer to map and add pressure on the TLBs.
I am aware that superpages will be useful for LiveUpdate, but is there 
any use cases in upstream? If not, could we just use the BUG_ON() and 
implement correctly vmap_to_mfn() in a follow-up?
Cheers,

--
Julien Grall