[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

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



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



 


Rackspace

Lists.xenproject.org is hosted with RackSpace, monitoring our
servers 24x7x365 and backed by RackSpace's Fanatical Support®.