> -----Original Message----- > From: Juergen Gross <jgr...@suse.com> > Sent: Thursday, May 19, 2022 10:31 AM > To: Lengyel, Tamas <tamas.leng...@intel.com>; xen- > de...@lists.xenproject.org > Cc: Wei Liu <w...@xen.org>; Anthony PERARD <anthony.per...@citrix.com>; > Cooper, Andrew <andrew.coop...@citrix.com> > Subject: Re: [PATCH] tools/libs/ctrl: add and export xc_memory_op > > On 19.05.22 15:59, Lengyel, Tamas wrote: > > > > > >> -----Original Message----- > >> From: Xen-devel <xen-devel-boun...@lists.xenproject.org> On Behalf Of > >> Juergen Gross > >> Sent: Thursday, May 19, 2022 9:33 AM > >> To: Lengyel, Tamas <tamas.leng...@intel.com>; > >> xen-devel@lists.xenproject.org > >> Cc: Wei Liu <w...@xen.org>; Anthony PERARD > <anthony.per...@citrix.com>; > >> Cooper, Andrew <andrew.coop...@citrix.com> > >> Subject: Re: [PATCH] tools/libs/ctrl: add and export xc_memory_op > >> > >> On 19.05.22 15:27, Tamas K Lengyel wrote: > >>> Add and export xc_memory_op so that do_memory_op can be used by > >>> tools linking with libxc. This is effectively in the same spirit as > >>> the existing xc_domctl and xc_sysctl functions, which are already > exported. > >>> > >>> In this patch we move do_memory_op into xc_private.h as a static > >>> inline function and convert its 'cmd' input from int to unsigned int > >>> to accurately reflect what the hypervisor expects. No other changes > >>> are made > >> to the function. > >>> > >>> Signed-off-by: Tamas K Lengyel <tamas.leng...@intel.com> > >>> --- > >>> tools/include/xenctrl.h | 1 + > >>> tools/libs/ctrl/xc_private.c | 63 +++--------------------------------- > >>> tools/libs/ctrl/xc_private.h | 58 ++++++++++++++++++++++++++++++++- > >>> 3 files changed, 63 insertions(+), 59 deletions(-) > >>> > >>> diff --git a/tools/include/xenctrl.h b/tools/include/xenctrl.h index > >>> 95bd5eca67..484e354412 100644 > >>> --- a/tools/include/xenctrl.h > >>> +++ b/tools/include/xenctrl.h > >>> @@ -1597,6 +1597,7 @@ int xc_vmtrace_set_option(xc_interface *xch, > >>> uint32_t domid, > >>> > >>> int xc_domctl(xc_interface *xch, struct xen_domctl *domctl); > >>> int xc_sysctl(xc_interface *xch, struct xen_sysctl *sysctl); > >>> +long xc_memory_op(xc_interface *xch, unsigned int cmd, void *arg, > >>> +size_t len); > >>> > >>> int xc_version(xc_interface *xch, int cmd, void *arg); > >>> > >>> diff --git a/tools/libs/ctrl/xc_private.c > >>> b/tools/libs/ctrl/xc_private.c index c0422662f0..6a247d2b1f 100644 > >>> --- a/tools/libs/ctrl/xc_private.c > >>> +++ b/tools/libs/ctrl/xc_private.c > >>> @@ -326,64 +326,6 @@ int xc_flush_mmu_updates(xc_interface *xch, > >>> struct > >> xc_mmu *mmu) > >>> return flush_mmu_updates(xch, mmu); > >>> } > >>> > >>> -long do_memory_op(xc_interface *xch, int cmd, void *arg, size_t > >>> len) > >> > >> Why don't you just rename this function and modify the users to use > >> the new name? > > > > For two reasons: > > 1) having the do_memory_op as a static inline is consistent with how > do_domctl and do_sysctl are implemented, so logically that's what I would > expect to see for the memory_op hypercall as well. > > It is much more complicated than the do_domctl and do_sysctl inlines. > > Additionally it is being used by libxenguest, so making it an inline would > expose lots of libxenctrl internals to libxenguest. > > > 2) the patch itself is cleaner because there is no churn in all the files > > that > previously called do_memory_op. > > OTOH all callers are in Xen, so its no deal to change those.
I'm fine with going the pure rename route if that's what's preferred. Tamas
