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

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@xxxxxxxxxxxxxxxxxxxx> On Behalf Of
Juergen Gross
Sent: Thursday, May 19, 2022 9:33 AM
To: Lengyel, Tamas <tamas.lengyel@xxxxxxxxx>; xen-devel@xxxxxxxxxxxxxxxxxxxx
Cc: Wei Liu <wl@xxxxxxx>; Anthony PERARD <anthony.perard@xxxxxxxxxx>;
Cooper, Andrew <andrew.cooper3@xxxxxxxxxx>
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@xxxxxxxxx>
---
   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

Attachment: OpenPGP_0xB0DE9DD628BF132F.asc
Description: OpenPGP public key

Attachment: OpenPGP_signature
Description: OpenPGP digital signature


 


Rackspace

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