Xen devel - Sep 2005 - Propose "xen_init()" call first thing in Xen drivers

Here''s another patch needed for ia64 in various xen drivers
that should probably be discussed before the actual patch
code is attached/applied.

Per previous posts, Xenlinux/ia64 is transparently paravirtualized,
which means the same bits run on top of Xen as on top of raw
iron.

To accomplish this, we have to be very careful to avoid doing
Xen-like things when we are not running on Xen.  The Xen
drivers tend to do Xen-like things early and often.  To avoid
the Xen drivers doing nasty things when not running on Xen,
we have added a single line at the beginning of various
drivers/xen/xxx xxx_init() routines, namely:

	if (xen_init() < 0) return -ENODEV

Where xen_init does "if (running_on_xen()) return -1;"
(among other things... see below) and where
running_on_xen() does the obvious (archdep) test.
We have been maintaining these changes out-of-tree.

Note that the routine need not be provided on x86 as
the line can be easily ifdef''d away by adding

	#define xen_init()	(0)

in a file such as asm-x86/hypervisor.h.

But wait... on xenlinux/ia64, xen_init does something
else!  In xenlinux/x86 there was (not sure if it is still
there) some funky linker stuff to ensure that the drivers
are linked in the right order as bad things happened
if they weren''t (due to the way init() works on Linux).
On xenlinux/ia64, xen_init() eliminates this problem with
a simple static one-shot variable in xen_init; regardless
of which routine calls xen_init first, xen_init does the
necessary initialization once.  No linker magic required.

So adding the xen_init test need not be ifdef''d away
for x86.

For xenlinux/ia64, we have the test added in a few
places, namely: xencons_init, gnttab_init, xenbus_probe_init.
It may also be needed elsewhere as we''ve been struggling
out-of-tree with keeping up with changes in driver/xen,
haven''t tested on raw iron in awhile, don''t have networking
working yet, etc.

So... how about we add the one line test to the init routines
of the various Xen drivers (including "core" drivers)?

Thanks,
Dan

P.S. The xen_init approach should be properly credited to
Matt Chapman.  I am just proposing it for the core tree.

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

No comments or objections?  If not, I will include
the (minimal) changes in the next ia64 merge which
I am preparing now. 
> -----Original Message-----
> From: Magenheimer, Dan (HP Labs Fort Collins) 
> Sent: Wednesday, September 21, 2005 3:51 PM
> To: xen-devel@lists.xensource.com
> Cc: ''Matt Chapman''
> Subject: Propose "xen_init()" call first thing in Xen drivers
> 
> Here''s another patch needed for ia64 in various xen drivers
> that should probably be discussed before the actual patch
> code is attached/applied.
> 
> Per previous posts, Xenlinux/ia64 is transparently paravirtualized,
> which means the same bits run on top of Xen as on top of raw
> iron.
> 
> To accomplish this, we have to be very careful to avoid doing
> Xen-like things when we are not running on Xen.  The Xen
> drivers tend to do Xen-like things early and often.  To avoid
> the Xen drivers doing nasty things when not running on Xen,
> we have added a single line at the beginning of various
> drivers/xen/xxx xxx_init() routines, namely:
> 
> 	if (xen_init() < 0) return -ENODEV
> 
> Where xen_init does "if (running_on_xen()) return -1;"
> (among other things... see below) and where
> running_on_xen() does the obvious (archdep) test.
> We have been maintaining these changes out-of-tree.
> 
> Note that the routine need not be provided on x86 as
> the line can be easily ifdef''d away by adding
> 
> 	#define xen_init()	(0)
> 
> in a file such as asm-x86/hypervisor.h.
> 
> But wait... on xenlinux/ia64, xen_init does something
> else!  In xenlinux/x86 there was (not sure if it is still
> there) some funky linker stuff to ensure that the drivers
> are linked in the right order as bad things happened
> if they weren''t (due to the way init() works on Linux).
> On xenlinux/ia64, xen_init() eliminates this problem with
> a simple static one-shot variable in xen_init; regardless
> of which routine calls xen_init first, xen_init does the
> necessary initialization once.  No linker magic required.
> 
> So adding the xen_init test need not be ifdef''d away
> for x86.
> 
> For xenlinux/ia64, we have the test added in a few
> places, namely: xencons_init, gnttab_init, xenbus_probe_init.
> It may also be needed elsewhere as we''ve been struggling
> out-of-tree with keeping up with changes in driver/xen,
> haven''t tested on raw iron in awhile, don''t have
networking
> working yet, etc.
> 
> So... how about we add the one line test to the init routines
> of the various Xen drivers (including "core" drivers)?
> 
> Thanks,
> Dan
> 
> P.S. The xen_init approach should be properly credited to
> Matt Chapman.  I am just proposing it for the core tree.
> 
_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xensource.com
http://lists.xensource.com/xen-devel

On Wednesday 21 September 2005 16:51, Magenheimer, Dan (HP Labs Fort Collins) 
wrote:
> Here''s another patch needed for ia64 in various xen drivers
> that should probably be discussed before the actual patch
> code is attached/applied.
>
> Per previous posts, Xenlinux/ia64 is transparently paravirtualized,
> which means the same bits run on top of Xen as on top of raw
> iron.
>
> To accomplish this, we have to be very careful to avoid doing
> Xen-like things when we are not running on Xen.  The Xen
> drivers tend to do Xen-like things early and often.  To avoid
> the Xen drivers doing nasty things when not running on Xen,
> we have added a single line at the beginning of various
> drivers/xen/xxx xxx_init() routines, namely:
>
> 	if (xen_init() < 0) return -ENODEV
I agree that something like this is necessary, though I don''t like the
"init"
direction you''re going in.

In general, drivers need some way of probing for their (virtual) device, and 
if the device is not present return ENODEV.

[...]
> But wait... on xenlinux/ia64, xen_init does something
> else!  In xenlinux/x86 there was (not sure if it is still
> there) some funky linker stuff to ensure that the drivers
> are linked in the right order as bad things happened
> if they weren''t (due to the way init() works on Linux).
> On xenlinux/ia64, xen_init() eliminates this problem with
> a simple static one-shot variable in xen_init; regardless
> of which routine calls xen_init first, xen_init does the
> necessary initialization once.  No linker magic required.
What linker magic exactly? I don''t understand the problem.

Could you provide a pointer to the ia64 xen_init() implementation? Just as PCI 
bus initialization takes place before PCI drivers are loaded, any Xen 
initialization should take place before the Xen drivers are loaded. PCI 
drivers do not call any pci_init() function.

-- 
Hollis Blanchard
IBM Linux Technology Center

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

> What linker magic exactly? I don''t understand the problem.
Linker magic is probably a poor choice of words.

IIRC, the problem arose because we moved xen/arch/xen/kernel
to -sparse/drivers/xen/core (which is planned as part of
the getting-into-the-linux process).  This changed the link
order which had previously (by chance) linked the evtchn_init
routine prior to any drivers/xen init routine.  If any
of the drivers/xen init routines execute before evtchn_init,
they fail.  Since we needed a test for "running_on_xen"
anyway, we (Matt) combined the two solutions into one architecturally
independent abstraction.
> Could you provide a pointer to the ia64 xen_init() 
> implementation? 
http://xenbits.xensource.com/ext/xenlinux-ia64-2.6.12.hg?cmd=file;fileno
de=769f15e75bbd990900b352a3f9d50562d9e399df;file=drivers/xen/core/xenia6
4_init.c

Thanks,
Dan

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

On Thursday 22 September 2005 14:20, Magenheimer, Dan (HP Labs Fort Collins) 
wrote:
> > What linker magic exactly? I don''t understand the problem.
>
> IIRC, the problem arose because we moved xen/arch/xen/kernel
> to -sparse/drivers/xen/core (which is planned as part of
> the getting-into-the-linux process).  This changed the link
> order which had previously (by chance) linked the evtchn_init
> routine prior to any drivers/xen init routine.  If any
> of the drivers/xen init routines execute before evtchn_init,
> they fail.  Since we needed a test for "running_on_xen"
> anyway, we (Matt) combined the two solutions into one architecturally
> independent abstraction.
Oh, well, there''s the problem: module_init(evtchn_init). That means 
evtchn_init will be called in arbitrary (link) order, intermingled with the 
actual drivers (which are also using module_init).

evtchn_init() can be built in and made a subsys_initcall, or there needs to be 
an explicit module dependency: e.g. request_module("evtchn").

http://xenbits.xensource.com/ext/xenlinux-ia64-2.6.12.hg?cmd=file;filenode=769f15e75bbd990900b352a3f9d50562d9e399df;file=drivers/xen/core/xenia64_init.c

As for xen_start_info in the above URL, why don''t you make sure arch
code sets
that up before drivers are loaded? You can turn your existing xen_init() into 
an arch_initcall.

-- 
Hollis Blanchard
IBM Linux Technology Center

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

> Oh, well, there''s the problem: module_init(evtchn_init). That
means
> evtchn_init will be called in arbitrary (link) order, 
> intermingled with the 
> actual drivers (which are also using module_init).
> 
> evtchn_init() can be built in and made a subsys_initcall, or 
> there needs to be 
> an explicit module dependency: e.g. request_module("evtchn").
> 
> http://xenbits.xensource.com/ext/xenlinux-ia64-2.6.12.hg?cmd=f
> ile;filenode=769f15e75bbd990900b352a3f9d50562d9e399df;file=dri
> vers/xen/core/xenia64_init.c
> 
> As for xen_start_info in the above URL, why don''t you make 
> sure arch code sets 
> that up before drivers are loaded? You can turn your existing 
> xen_init() into 
> an arch_initcall.
Thanks much for the ideas.  We will have a look at this
again (hopefully soon) but are currently broiling bigger
fish, so the current "workaround" is sufficient for now.

Dan

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