| Message ID | 5c72f793978997970888254a9050e97b34cbb3c7.1652966447.git.tamas.lengyel@intel.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | tools/libs/ctrl: add and export xc_memory_op | expand |
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.lengyel@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? Juergen
> -----Original Message----- > From: Xen-devel <xen-devel-bounces@lists.xenproject.org> On Behalf Of > Juergen Gross > Sent: Thursday, May 19, 2022 9:33 AM > To: Lengyel, Tamas <tamas.lengyel@intel.com>; xen-devel@lists.xenproject.org > Cc: Wei Liu <wl@xen.org>; Anthony PERARD <anthony.perard@citrix.com>; > Cooper, Andrew <andrew.cooper3@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.lengyel@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. 2) the patch itself is cleaner because there is no churn in all the files that previously called do_memory_op. Tamas
On 19.05.22 15:59, Lengyel, Tamas wrote: > > >> -----Original Message----- >> From: Xen-devel <xen-devel-bounces@lists.xenproject.org> On Behalf Of >> Juergen Gross >> Sent: Thursday, May 19, 2022 9:33 AM >> To: Lengyel, Tamas <tamas.lengyel@intel.com>; xen-devel@lists.xenproject.org >> Cc: Wei Liu <wl@xen.org>; Anthony PERARD <anthony.perard@citrix.com>; >> Cooper, Andrew <andrew.cooper3@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.lengyel@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. Juergen
> -----Original Message----- > From: Juergen Gross <jgross@suse.com> > Sent: Thursday, May 19, 2022 10:31 AM > To: Lengyel, Tamas <tamas.lengyel@intel.com>; xen- > devel@lists.xenproject.org > Cc: Wei Liu <wl@xen.org>; Anthony PERARD <anthony.perard@citrix.com>; > Cooper, Andrew <andrew.cooper3@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-bounces@lists.xenproject.org> On Behalf Of > >> Juergen Gross > >> Sent: Thursday, May 19, 2022 9:33 AM > >> To: Lengyel, Tamas <tamas.lengyel@intel.com>; > >> xen-devel@lists.xenproject.org > >> Cc: Wei Liu <wl@xen.org>; Anthony PERARD > <anthony.perard@citrix.com>; > >> Cooper, Andrew <andrew.cooper3@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.lengyel@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
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) -{ - DECLARE_HYPERCALL_BOUNCE(arg, len, XC_HYPERCALL_BUFFER_BOUNCE_BOTH); - long ret = -1; - - if ( xc_hypercall_bounce_pre(xch, arg) ) - { - PERROR("Could not bounce memory for XENMEM hypercall"); - goto out1; - } - -#if defined(__linux__) || defined(__sun__) - /* - * Some sub-ops return values which don't fit in "int". On platforms - * without a specific hypercall return value field in the privcmd - * interface structure, issue the request as a single-element multicall, - * to be able to capture the full return value. - */ - if ( sizeof(long) > sizeof(int) ) - { - multicall_entry_t multicall = { - .op = __HYPERVISOR_memory_op, - .args[0] = cmd, - .args[1] = HYPERCALL_BUFFER_AS_ARG(arg), - }, *call = &multicall; - DECLARE_HYPERCALL_BOUNCE(call, sizeof(*call), - XC_HYPERCALL_BUFFER_BOUNCE_BOTH); - - if ( xc_hypercall_bounce_pre(xch, call) ) - { - PERROR("Could not bounce buffer for memory_op hypercall"); - goto out1; - } - - ret = do_multicall_op(xch, HYPERCALL_BUFFER(call), 1); - - xc_hypercall_bounce_post(xch, call); - - if ( !ret ) - { - ret = multicall.result; - if ( multicall.result > ~0xfffUL ) - { - errno = -ret; - ret = -1; - } - } - } - else -#endif - ret = xencall2L(xch->xcall, __HYPERVISOR_memory_op, - cmd, HYPERCALL_BUFFER_AS_ARG(arg)); - - xc_hypercall_bounce_post(xch, arg); - out1: - return ret; -} - int xc_maximum_ram_page(xc_interface *xch, unsigned long *max_mfn) { long rc = do_memory_op(xch, XENMEM_maximum_ram_page, NULL, 0); @@ -489,6 +431,11 @@ int xc_sysctl(xc_interface *xch, struct xen_sysctl *sysctl) return do_sysctl(xch, sysctl); } +long xc_memory_op(xc_interface *xch, unsigned int cmd, void *arg, size_t len) +{ + return do_memory_op(xch, cmd, arg, len); +} + int xc_version(xc_interface *xch, int cmd, void *arg) { DECLARE_HYPERCALL_BOUNCE(arg, 0, XC_HYPERCALL_BUFFER_BOUNCE_OUT); /* Size unknown until cmd decoded */ diff --git a/tools/libs/ctrl/xc_private.h b/tools/libs/ctrl/xc_private.h index ebdf78c2bf..cf6ad932b0 100644 --- a/tools/libs/ctrl/xc_private.h +++ b/tools/libs/ctrl/xc_private.h @@ -367,7 +367,63 @@ static inline int do_multicall_op(xc_interface *xch, return ret; } -long do_memory_op(xc_interface *xch, int cmd, void *arg, size_t len); +static inline long do_memory_op(xc_interface *xch, unsigned int cmd, void *arg, size_t len) +{ + DECLARE_HYPERCALL_BOUNCE(arg, len, XC_HYPERCALL_BUFFER_BOUNCE_BOTH); + long ret = -1; + + if ( xc_hypercall_bounce_pre(xch, arg) ) + { + PERROR("Could not bounce memory for XENMEM hypercall"); + goto out1; + } + +#if defined(__linux__) || defined(__sun__) + /* + * Some sub-ops return values which don't fit in "int". On platforms + * without a specific hypercall return value field in the privcmd + * interface structure, issue the request as a single-element multicall, + * to be able to capture the full return value. + */ + if ( sizeof(long) > sizeof(int) ) + { + multicall_entry_t multicall = { + .op = __HYPERVISOR_memory_op, + .args[0] = cmd, + .args[1] = HYPERCALL_BUFFER_AS_ARG(arg), + }, *call = &multicall; + DECLARE_HYPERCALL_BOUNCE(call, sizeof(*call), + XC_HYPERCALL_BUFFER_BOUNCE_BOTH); + + if ( xc_hypercall_bounce_pre(xch, call) ) + { + PERROR("Could not bounce buffer for memory_op hypercall"); + goto out1; + } + + ret = do_multicall_op(xch, HYPERCALL_BUFFER(call), 1); + + xc_hypercall_bounce_post(xch, call); + + if ( !ret ) + { + ret = multicall.result; + if ( multicall.result > ~0xfffUL ) + { + errno = -ret; + ret = -1; + } + } + } + else +#endif + ret = xencall2L(xch->xcall, __HYPERVISOR_memory_op, + cmd, HYPERCALL_BUFFER_AS_ARG(arg)); + + xc_hypercall_bounce_post(xch, arg); + out1: + return ret; +} void *xc_map_foreign_ranges(xc_interface *xch, uint32_t dom, size_t size, int prot, size_t chunksize,
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.lengyel@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(-)