Mailing List Archive

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,

Alex

On Wed, 2008-02-27 at 16:01 +0900, Simon Horman wrote:
> Hi,
>
> this series starts off by reworking the hypercall a bit to
> allow it to have architecture-specific code under xen/arch/
>
> It then extends the hypercall for some extra regions that
> are needed for xen ia64.
>
> There are generic and ia64 specific patches in this series.
> I wanted to post them together as they don't make much sense
> without each other.
>
> There are related xen-linux patches that I will send as a separate series.
> There are related kexec-tools patches which I have posted to
> the kexec mailing list and intend to merge.
> http://lists.infradead.org/pipermail/kexec/2008-February/001348.html
>
>
> The end-game here is to make sure that all the reserved regions
> show up in /proc/iomem_machine on ia64. This currently does not happen.
> And manifests as kexec only being able to be performed once as
> the boot parameter ends up being overwritten in relocate_kernel before
> purgatory is reached.
>
> Xen--kexec-->Xen--kexec [hang in purgatory!]
>
>
> The EFI-RID patches for ia64 xen kexec are also needed for kexec
> to function on ia64. However they touch different code paths and can
> be merged separately.
>
--
Alex Williamson HP Open Source & Linux Org.


_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xensource.com
http://lists.xensource.com/xen-devel

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"
#endif

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
either

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.

--
Horms


_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xensource.com
http://lists.xensource.com/xen-devel

On 28/2/08 03:35, "Simon Horman" <horms@verge.net.au> wrote:

> 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.

You're not alone. ;-) However, it does work and anybody competent to clean
it up seems to have plenty of other stuff to do!

-- Keir



_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xensource.com
http://lists.xensource.com/xen-devel

On Thu, 2008-02-28 at 12:35 +0900, Simon Horman wrote:
> 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.

Wow, that's pretty nasty. Thanks for the explanation.

Alex

--
Alex Williamson HP Open Source & Linux Org.


_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xensource.com
http://lists.xensource.com/xen-devel

On Thu, Feb 28, 2008 at 08:55:18AM +0000, Keir Fraser wrote:
> On 28/2/08 03:35, "Simon Horman" <horms@verge.net.au> wrote:
>
> > 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.
>
> You're not alone. ;-) However, it does work and anybody competent to clean
> it up seems to have plenty of other stuff to do!

I'm more than happy to clean it up if there is a chance the patches
will be accepted. I'll start working on that. But can we treat
that separately to this series?

--
Horms


_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xensource.com
http://lists.xensource.com/xen-devel