This is an archived copy of the Xen.org mailing list, which we have preserved to ensure that existing links to archives are not broken. The live archive, which contains the latest emails, can be found at http://lists.xen.org/
Home Products Support Community News


[Xen-devel] Re: [patch 0/3] xen: Extend xen kexec hypercall to return ad

To: Alex Williamson <alex.williamson@xxxxxx>
Subject: [Xen-devel] Re: [patch 0/3] xen: Extend xen kexec hypercall to return additional regions
From: Simon Horman <horms@xxxxxxxxxxxx>
Date: Thu, 28 Feb 2008 12:35:18 +0900
Cc: xen-devel@xxxxxxxxxxxxxxxxxxx, Ian Campbell <Ian.Campbell@xxxxxxxxxx>, xen-ia64-devel@xxxxxxxxxxxxxxxxxxx
Delivery-date: Wed, 27 Feb 2008 19:35:48 -0800
Envelope-to: www-data@xxxxxxxxxxxxxxxxxx
In-reply-to: <1204142152.6798.32.camel@lappy>
List-help: <mailto:xen-devel-request@lists.xensource.com?subject=help>
List-id: Xen developer discussion <xen-devel.lists.xensource.com>
List-post: <mailto:xen-devel@lists.xensource.com>
List-subscribe: <http://lists.xensource.com/cgi-bin/mailman/listinfo/xen-devel>, <mailto:xen-devel-request@lists.xensource.com?subject=subscribe>
List-unsubscribe: <http://lists.xensource.com/cgi-bin/mailman/listinfo/xen-devel>, <mailto:xen-devel-request@lists.xensource.com?subject=unsubscribe>
References: <20080227070105.949799153@xxxxxxxxxxxx> <1204142152.6798.32.camel@lappy>
Sender: xen-devel-bounces@xxxxxxxxxxxxxxxxxxx
User-agent: Mutt/1.5.17+20080114 (2008-01-14)
On Wed, Feb 27, 2008 at 12:55:52PM -0700, Alex Williamson wrote:
> Hi Simon,
>    I'm not seeing how this doesn't break the x86 COMPAT/CONFIG_COMPAT
> code paths.  Where does kexec_get_range_internal() get defined for
> COMPAT in [1/3], and where is machine_kexec_get_xen() defined for
> CONFIG_COMPAT in [2/3]?  I'm fine with the ia64 parts if Ian/Keir want
> to check them into the main tree, but there is some mixed indenting in
> [3/3].  Thanks,

Thanks for pointing out the white-space issue, I'll fix that up asap.

As for the compat stuff...

The way that I understand the code is that basically
xen/common/kexec.c gets compiled twice using the following mechanism
at the very bottom of the file:

#if defined(CONFIG_COMPAT) && !defined(COMPAT)
#include "compat/kexec.c"

Where xen/common/compat/kexec.c looks a bit like this:

#define COMPAT
#include "../kexec.c"

So on the first stage of the run through xen/common/kexec.c, before
we get to #include "compat/kexec.c", any code that
is not protected by #ifdef COMPAT will be compiled.

If CONFIG_COMPAT isn't defined (elsewhere) then that is the
end and all is well.

If CONFIG_COMPAT is defined, then we enter the second stage,
where any code in "compat/kexec.c" not protected by #ifndef COMPAT
is compiled.

The important thing to note is that during this second stage,
all the code compiled in the first stage still exists.

kexec_get_range_internal() and all the functions that it calls

a) exist in xen/common/kexec.c and are compiled during the first stage,
   that is, they are protected by #ifndef COMPAT.

b) present in other files that and are compiled independent
   of any CONFIG_COMPAT games.

This means that kexec_get_range_internal(), and all the functions
that it relies on can be called by both non-compat version 
of do_kexec_op(), which is compiled during the first stage,
and the compat version of do_kexec_op() (compat_kexec_op()).

The creation of the first version of do_kexec_op() is fairly obvious,
it is compiled as you would expect on a casual glance. This occurs
during stage one. The compilation of compat_kexec_op() is achieved
by "#define do_kexec_op compat_kexec_op" in xen/common/compat/kexec.c,
which essentially changes the function name for compilation in the
second phase.

I'll come right out and say that I don't fancy this two stage .c
indirectly including itself approach. And I hope this change can help us
move away from there, as well as providing the functionality
extendability that this patch set describes.

As to the extent of testing this code. I have run it on ia64 and it
works. I have also compiled it on both i386 and x86_64. The latter
uses CONFIG_COMPAT by default, which is the configuration I tested.


Xen-devel mailing list