Re: [Xen-devel] [PATCH] xen/common: Drop function calls for Xen compile/version information

[Konrad Rzeszutek Wilk <konrad.wilk@xxxxxxxxxx>]

On Tue, Jan 17, 2017 at 01:42:54PM -0500, Konrad Rzeszutek Wilk wrote:
> On Tue, Jan 17, 2017 at 06:16:36PM +0000, Andrew Cooper wrote:
> > On 17/01/17 18:05, Konrad Rzeszutek Wilk wrote:
> > > On Mon, Jan 16, 2017 at 01:04:09PM +0000, Andrew Cooper wrote:
> > >> The chageset/version/compile information is currently exported as a set 
> > >> of
> > >> function calls into a separate translation unit, which is inefficient 
> > >> for all
> > >> callers.
> > >>
> > >> Replace the function calls with externs pointing appropriately into 
> > >> .rodata,
> > >> which allows all users to generate code referencing the data directly.
> > >>
> > >> No functional change, but causes smaller and more efficient compiled 
> > >> code.
> > >>
> > >> Signed-off-by: Andrew Cooper <andrew.cooper3@xxxxxxxxxx>
> > > Ah crud. That breaks the livepatch test-cases (they patch the 
> > > xen_extra_version
> > > function).
> > 
> > Lucky I haven't pushed it then, (although the livepatch build seems to
> > still work fine for me, despite this change.)
> 
> make tests should fail.

(which is not by built by default as requested by Jan - but the
OSSTest test cases I am working would do this).
> 
> > 
> > > Are there some other code that can be modified that is reported
> > > by 'xl info' on which the test-cases can run (and reported easily?).
> > 
> > Patch do_version() itself to return the same difference of information?
> 
> Ugh. That is going to make the building of test-cases quite complex.
> 
> I guess it can just do it and .. return only one value :-)

As in something like this (not compile tested):


diff --git a/xen/arch/x86/test/xen_hello_world_func.c 
b/xen/arch/x86/test/xen_hello_world_func.c
index 2e4af9c..3572600 100644
--- a/xen/arch/x86/test/xen_hello_world_func.c
+++ b/xen/arch/x86/test/xen_hello_world_func.c
@@ -10,7 +10,7 @@
 static unsigned long *non_canonical_addr = (unsigned long 
*)0xdead000000000000ULL;
 
 /* Our replacement function for xen_extra_version. */
-const char *xen_hello_world(void)
+const char *xen_version_hello_world(int cmd, XEN_GUEST_HANDLE_PARAM(void) arg)
 {
     unsigned long tmp;
     int rc;
@@ -21,7 +21,19 @@ const char *xen_hello_world(void)
     rc = __get_user(tmp, non_canonical_addr);
     BUG_ON(rc != -EFAULT);
 
-    return "Hello World";
+    if ( cmd == XENVER_extraversion )
+    {
+        xen_extraversion_t extraversion = "Hello World";
+
+        if ( copy_to_guest(arg, extraversion, ARRAY_SIZE(extraversion)) )
+            return -EFAULT;
+        return 0;
+    }
+    /*
+     * Can't return -EPERM as certain subversions can't deal with negative
+     * values.
+     */
+    return 0;
 }


That will make three of the test-patches work but not for the last
one - the NOP one (which needs to patch the _whole_ function).

Argh.

Is this patch of yours that neccessary? Could at least some of the
functions still exist?

Like 

xen_minor_version() and xen_extra_verison() ?

_______________________________________________
Xen-devel mailing list
Xen-devel@xxxxxxxxxxxxx
https://lists.xen.org/xen-devel