Re: [PATCH v4 1/4] xen: XENMEM_exchange should only be used/compiled for arch supporting PV guest

[Julien Grall <julien@xxxxxxx>]

Hi Jan,

On 22/09/2020 08:35, Jan Beulich wrote:
> On 21.09.2020 20:02, Julien Grall wrote:
> > --- a/xen/common/memory.c
> > +++ b/xen/common/memory.c
> > @@ -504,6 +504,7 @@ static bool propagate_node(unsigned int xmf, unsigned int 
> > *memflags)
> >   
> >   static long memory_exchange(XEN_GUEST_HANDLE_PARAM(xen_memory_exchange_t) arg)
> >   {
> > +#ifdef CONFIG_PV
> >       struct xen_memory_exchange exch;
> >       PAGE_LIST_HEAD(in_chunk_list);
> >       PAGE_LIST_HEAD(out_chunk_list);
> > @@ -516,6 +517,9 @@ static long 
> > memory_exchange(XEN_GUEST_HANDLE_PARAM(xen_memory_exchange_t) arg)
> >       struct domain *d;
> >       struct page_info *page;
> >   
> > +    if ( !is_pv_domain(d) )
> > +        return -EOPNOTSUPP;
> I think "paging_mode_translate(d)" would be more correct, at which
> point the use later in the function ought to be dropped (as that's
> precisely one of the things making this function not really
> sensible to use on translated guests).
I have used paging_mode_translate(d) and remove the later use.

> I also wonder whether the #ifdef wouldn't better be left out. Yes,
> that'll mean retaining a stub mfn_to_gmfn(), but it could expand
> to simply BUG() then, for example.
Keeping mfn_to_gmfn() will not discourage developper to add more use of 
it. The risk is we don't notice it when reviewing  and this would lead 
to hit a BUG_ON().
Given this will be the only place where mfn_to_gmfn() is used, I think 
it is best to stub the code. Bear in mind that long term, this function 
should leave outside of common/mm.c (see Andrew's e-mail).
Cheers,

--
Julien Grall