All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH]: Prevent sn2 ptc code from executing on all ia64 subarches
@ 2005-11-21 18:00 Prarit Bhargava
  2005-12-09 17:11 ` Robin Holt
                   ` (23 more replies)
  0 siblings, 24 replies; 25+ messages in thread
From: Prarit Bhargava @ 2005-11-21 18:00 UTC (permalink / raw)
  To: linux-ia64

Patch to prevent sn2_ptc_init code from attempting to load on non-sn2 systems
when sn2_smp.c is built-in to kernel.

Signed-off-by: Prarit Bhargava <prarit@sgi.com>

diff --git a/arch/ia64/sn/kernel/sn2/sn2_smp.c b/arch/ia64/sn/kernel/sn2/sn2_smp.c
--- a/arch/ia64/sn/kernel/sn2/sn2_smp.c
+++ b/arch/ia64/sn/kernel/sn2/sn2_smp.c
@@ -492,6 +492,9 @@ static struct proc_dir_entry *proc_sn2_p
 
 static int __init sn2_ptc_init(void)
 {
+	if (!ia64_platform_is("sn2"))
+		return -ENOSYS;
+
 	if (!(proc_sn2_ptc = create_proc_entry(PTC_BASENAME, 0444, NULL))) {
 		printk(KERN_ERR "unable to create %s proc entry", PTC_BASENAME);
 		return -EINVAL;

^ permalink raw reply	[flat|nested] 25+ messages in thread

* Re: [PATCH]: Prevent sn2 ptc code from executing on all ia64 subarches
  2005-11-21 18:00 [PATCH]: Prevent sn2 ptc code from executing on all ia64 subarches Prarit Bhargava
@ 2005-12-09 17:11 ` Robin Holt
  2005-12-09 17:46 ` Bjorn Helgaas
                   ` (22 subsequent siblings)
  23 siblings, 0 replies; 25+ messages in thread
From: Robin Holt @ 2005-12-09 17:11 UTC (permalink / raw)
  To: linux-ia64

Prarit, I don't think your patch catches all the cases.  Try
this one.

Index: linux-2.6/arch/ia64/sn/kernel/sn2/sn_hwperf.c
=================================--- linux-2.6.orig/arch/ia64/sn/kernel/sn2/sn_hwperf.c	2005-11-21 20:05:40.000000000 -0600
+++ linux-2.6/arch/ia64/sn/kernel/sn2/sn_hwperf.c	2005-12-09 10:53:19.159948227 -0600
@@ -973,6 +973,9 @@ static int __devinit sn_hwperf_misc_regi
 {
 	int e;
 
+	if (!ia64_platform_is("sn2"))
+		return -ENODEV;
+
 	sn_hwperf_init();
 
 	/*
Index: linux-2.6/arch/ia64/sn/kernel/mca.c
=================================--- linux-2.6.orig/arch/ia64/sn/kernel/mca.c	2005-11-09 10:53:59.000000000 -0600
+++ linux-2.6/arch/ia64/sn/kernel/mca.c	2005-12-09 10:49:52.089585038 -0600
@@ -136,6 +136,9 @@ int sn_salinfo_platform_oemdata(const u8
 
 static int __init sn_salinfo_init(void)
 {
+	if (!ia64_platform_is("sn2"))
+		return -ENODEV;
+
 	salinfo_platform_oemdata = &sn_salinfo_platform_oemdata;
 	return 0;
 }
Index: linux-2.6/drivers/char/snsc.c
=================================--- linux-2.6.orig/drivers/char/snsc.c	2005-11-09 10:54:02.000000000 -0600
+++ linux-2.6/drivers/char/snsc.c	2005-12-09 10:54:48.619864496 -0600
@@ -375,7 +375,12 @@ scdrv_init(void)
 	struct sysctl_data_s *scd;
 	void *salbuf;
 	dev_t first_dev, dev;
-	nasid_t event_nasid = ia64_sn_get_console_nasid();
+	nasid_t event_nasid;
+
+	if (!ia64_platform_is("sn2"))
+		return -ENODEV;
+
+	event_nasid = ia64_sn_get_console_nasid();
 
 	if (alloc_chrdev_region(&first_dev, 0, num_cnodes,
 				SYSCTL_BASENAME) < 0) {
Index: linux-2.6/drivers/pci/hotplug/sgi_hotplug.c
=================================--- linux-2.6.orig/drivers/pci/hotplug/sgi_hotplug.c	2005-11-09 10:54:05.000000000 -0600
+++ linux-2.6/drivers/pci/hotplug/sgi_hotplug.c	2005-12-09 10:49:46.197589209 -0600
@@ -552,6 +552,9 @@ static int sn_pci_hotplug_init(void)
 	int rc;
 	int registered = 0;
 
+	if (!ia64_platform_is("sn2"))
+		return -ENODEV;
+
 	if (sn_sal_rev() < SGI_HOTPLUG_PROM_REV) {
 		printk(KERN_ERR "%s: PROM version must be greater than 4.30\n",
 		       __FUNCTION__);
Index: linux-2.6/drivers/sn/ioc4.c
=================================--- linux-2.6.orig/drivers/sn/ioc4.c	2005-11-09 10:54:06.000000000 -0600
+++ linux-2.6/drivers/sn/ioc4.c	2005-12-09 10:51:18.627908933 -0600
@@ -406,6 +406,9 @@ MODULE_DEVICE_TABLE(pci, ioc4_id_table);
 static int __devinit
 ioc4_init(void)
 {
+	if (!ia64_platform_is("sn2"))
+		return -ENODEV;
+
 	return pci_register_driver(&ioc4_driver);
 }
 

^ permalink raw reply	[flat|nested] 25+ messages in thread

* Re: [PATCH]: Prevent sn2 ptc code from executing on all ia64 subarches
  2005-11-21 18:00 [PATCH]: Prevent sn2 ptc code from executing on all ia64 subarches Prarit Bhargava
  2005-12-09 17:11 ` Robin Holt
@ 2005-12-09 17:46 ` Bjorn Helgaas
  2005-12-09 17:54 ` Luck, Tony
                   ` (21 subsequent siblings)
  23 siblings, 0 replies; 25+ messages in thread
From: Bjorn Helgaas @ 2005-12-09 17:46 UTC (permalink / raw)
  To: linux-ia64

On Friday 09 December 2005 10:11 am, Robin Holt wrote:
> Prarit, I don't think your patch catches all the cases.  Try
> this one.
> 
> Index: linux-2.6/arch/ia64/sn/kernel/sn2/sn_hwperf.c
> =================================> --- linux-2.6.orig/arch/ia64/sn/kernel/sn2/sn_hwperf.c	2005-11-21 20:05:40.000000000 -0600
> +++ linux-2.6/arch/ia64/sn/kernel/sn2/sn_hwperf.c	2005-12-09 10:53:19.159948227 -0600
> @@ -973,6 +973,9 @@ static int __devinit sn_hwperf_misc_regi
>  {
>  	int e;
>  
> +	if (!ia64_platform_is("sn2"))
> +		return -ENODEV;

I'm not thrilled about this approach.

I'd *like* to be able to assume that "changes in arch/ia64/sn/* clearly
don't affect non-SN platforms".  But this style breaks that.  Every ia64
box calls all these SN init functions, and if somebody forgets the
ia64_platform_is("sn2") check, bad things will happen.

I'd like it a whole lot better if all these initialization-type things
could be hidden inside sn_setup() or some other machine vector.


^ permalink raw reply	[flat|nested] 25+ messages in thread

* RE: [PATCH]: Prevent sn2 ptc code from executing on all ia64 subarches
  2005-11-21 18:00 [PATCH]: Prevent sn2 ptc code from executing on all ia64 subarches Prarit Bhargava
  2005-12-09 17:11 ` Robin Holt
  2005-12-09 17:46 ` Bjorn Helgaas
@ 2005-12-09 17:54 ` Luck, Tony
  2005-12-09 18:22 ` Prarit Bhargava
                   ` (20 subsequent siblings)
  23 siblings, 0 replies; 25+ messages in thread
From: Luck, Tony @ 2005-12-09 17:54 UTC (permalink / raw)
  To: linux-ia64

> I'm not thrilled about this approach.
> 
> I'd *like* to be able to assume that "changes in arch/ia64/sn/* clearly
> don't affect non-SN platforms".  But this style breaks that.  Every ia64
> box calls all these SN init functions, and if somebody forgets the
> ia64_platform_is("sn2") check, bad things will happen.
> 
> I'd like it a whole lot better if all these initialization-type things
> could be hidden inside sn_setup() or some other machine vector.

Me too ... these ia64_platform_is("sn2") are getting sprinkled in
more and more places.

-Tony

^ permalink raw reply	[flat|nested] 25+ messages in thread

* Re: [PATCH]: Prevent sn2 ptc code from executing on all ia64 subarches
  2005-11-21 18:00 [PATCH]: Prevent sn2 ptc code from executing on all ia64 subarches Prarit Bhargava
                   ` (2 preceding siblings ...)
  2005-12-09 17:54 ` Luck, Tony
@ 2005-12-09 18:22 ` Prarit Bhargava
  2005-12-09 18:31 ` Prarit Bhargava
                   ` (19 subsequent siblings)
  23 siblings, 0 replies; 25+ messages in thread
From: Prarit Bhargava @ 2005-12-09 18:22 UTC (permalink / raw)
  To: linux-ia64

Luck, Tony wrote:
>>I'm not thrilled about this approach.
>>
>>I'd *like* to be able to assume that "changes in arch/ia64/sn/* clearly
>>don't affect non-SN platforms".  But this style breaks that.  Every ia64
>>box calls all these SN init functions, and if somebody forgets the
>>ia64_platform_is("sn2") check, bad things will happen.
>>
>>I'd like it a whole lot better if all these initialization-type things
>>could be hidden inside sn_setup() or some other machine vector.

The issue is that the initcalls are not executed at the same point in the boot 
-- there are seven levels of these and each level needs to be honoured. 
Placing them inside sn_setup() or some other machine vector is not the right 
thing to do (IMO -- there might be away to code around the levels that I'm 
unaware of).

> 
> 
> Me too ... these ia64_platform_is("sn2") are getting sprinkled in
> more and more places.
> 

I've been thinking about this and am wondering if the following solution might 
be more acceptable?

I'm also concerned about plugging in many ia64_platform_is checks into the code 
-- as Tony and Bjorn point out it seems that this approach is fraught with 
peril.  I'm also concerned about the possiblity of executing non-SGI code on one 
of my systems.

What if we added wrappers to the existing initcall functions to accept another 
argument?

Instead of subsys_initcall(fn) we would do

subarch_subsys_initcall (fn, arch) and this wrapper would do a platform check of 
the machine vector info?

ie)

subarch_subsys_initcall (init_foo, sn2)?


This seems a lot cleaner and would clean up other modular code as well.

I haven't completed my investigation yet -- the first patch that I submitted was 
in response to (what appeared to be) a minor issue reported by a colleague at 
another vendor and as Robin points out, is clearly only the start of the fix.

P.


^ permalink raw reply	[flat|nested] 25+ messages in thread

* Re: [PATCH]: Prevent sn2 ptc code from executing on all ia64 subarches
  2005-11-21 18:00 [PATCH]: Prevent sn2 ptc code from executing on all ia64 subarches Prarit Bhargava
                   ` (3 preceding siblings ...)
  2005-12-09 18:22 ` Prarit Bhargava
@ 2005-12-09 18:31 ` Prarit Bhargava
  2005-12-09 18:38 ` Luck, Tony
                   ` (18 subsequent siblings)
  23 siblings, 0 replies; 25+ messages in thread
From: Prarit Bhargava @ 2005-12-09 18:31 UTC (permalink / raw)
  To: linux-ia64


> I haven't completed my investigation yet -- the first patch that I 
> submitted was in response to (what appeared to be) a minor issue 
> reported by a colleague at another vendor and as Robin points out, is 
> clearly only the start of the fix.
> 

Here I am replying to myself :p

hp/sim/simserial.c:1038:__initcall(simrs_init);
hp/sim/simeth.c:532:__initcall(simeth_probe);

This code has the ia64_platform_is wrapper -- and could be mod'ded as previously 
suggested.

P.

> P.
> 
> 


^ permalink raw reply	[flat|nested] 25+ messages in thread

* RE: [PATCH]: Prevent sn2 ptc code from executing on all ia64 subarches
  2005-11-21 18:00 [PATCH]: Prevent sn2 ptc code from executing on all ia64 subarches Prarit Bhargava
                   ` (4 preceding siblings ...)
  2005-12-09 18:31 ` Prarit Bhargava
@ 2005-12-09 18:38 ` Luck, Tony
  2005-12-09 19:41 ` Andreas Schwab
                   ` (17 subsequent siblings)
  23 siblings, 0 replies; 25+ messages in thread
From: Luck, Tony @ 2005-12-09 18:38 UTC (permalink / raw)
  To: linux-ia64

> What if we added wrappers to the existing initcall functions to accept another 
> argument?
> 
> Instead of subsys_initcall(fn) we would do
> 
> subarch_subsys_initcall (fn, arch) and this wrapper would do a platform check of 
> the machine vector info?

That looks like a plausible direction to investigate.  Does anyone know
whether any other architectures have a similar problem that can be solved
in this way?  Or is ia64 the only one with a "generic" kernel that has to
work on such a diverse spread of machines?

Prarit: when you get further with this you'll need to post to linux-kernel
as well as linux-ia64 since this will touch generic code.

-Tony

^ permalink raw reply	[flat|nested] 25+ messages in thread

* Re: [PATCH]: Prevent sn2 ptc code from executing on all ia64 subarches
  2005-11-21 18:00 [PATCH]: Prevent sn2 ptc code from executing on all ia64 subarches Prarit Bhargava
                   ` (5 preceding siblings ...)
  2005-12-09 18:38 ` Luck, Tony
@ 2005-12-09 19:41 ` Andreas Schwab
  2005-12-13 14:06 ` Prarit Bhargava
                   ` (16 subsequent siblings)
  23 siblings, 0 replies; 25+ messages in thread
From: Andreas Schwab @ 2005-12-09 19:41 UTC (permalink / raw)
  To: linux-ia64

"Luck, Tony" <tony.luck@intel.com> writes:

> That looks like a plausible direction to investigate.  Does anyone know
> whether any other architectures have a similar problem that can be solved
> in this way?  Or is ia64 the only one with a "generic" kernel that has to
> work on such a diverse spread of machines?

PPC has CONFIG_PPC_MULTIPLATFORM, and use ppc_md much like ia64 uses
ia64_mv.

Andreas.

-- 
Andreas Schwab, SuSE Labs, schwab@suse.de
SuSE Linux Products GmbH, Maxfeldstraße 5, 90409 Nürnberg, Germany
PGP key fingerprint = 58CA 54C7 6D53 942B 1756  01D3 44D5 214B 8276 4ED5
"And now for something completely different."

^ permalink raw reply	[flat|nested] 25+ messages in thread

* Re: [PATCH]: Prevent sn2 ptc code from executing on all ia64 subarches
  2005-11-21 18:00 [PATCH]: Prevent sn2 ptc code from executing on all ia64 subarches Prarit Bhargava
                   ` (6 preceding siblings ...)
  2005-12-09 19:41 ` Andreas Schwab
@ 2005-12-13 14:06 ` Prarit Bhargava
  2005-12-13 15:10 ` Andreas Schwab
                   ` (15 subsequent siblings)
  23 siblings, 0 replies; 25+ messages in thread
From: Prarit Bhargava @ 2005-12-13 14:06 UTC (permalink / raw)
  To: linux-ia64

Andreas Schwab wrote:
> "Luck, Tony" <tony.luck@intel.com> writes:
> 
> 
>>That looks like a plausible direction to investigate.  Does anyone know
>>whether any other architectures have a similar problem that can be solved
>>in this way?  Or is ia64 the only one with a "generic" kernel that has to
>>work on such a diverse spread of machines?
> 
> 
> PPC has CONFIG_PPC_MULTIPLATFORM, and use ppc_md much like ia64 uses
> ia64_mv.
> 

Andreas -- I just recieved the following from a PPC engineer at IBM:

"I don't think it is possible to build a ppc kernel with just
CONFIG_PPC_MULTIPLATFORM set. You'll need to "choose" another compile
option such as "cell" or "iseries" or "pseries" or somesuch (from
arch/powerpc/platforms/). Check arch/powerpc/configs for the relevant
defconfigs."

... so it looks like this is currently limited to ia64 ...

P.

> Andreas.
> 


^ permalink raw reply	[flat|nested] 25+ messages in thread

* Re: [PATCH]: Prevent sn2 ptc code from executing on all ia64 subarches
  2005-11-21 18:00 [PATCH]: Prevent sn2 ptc code from executing on all ia64 subarches Prarit Bhargava
                   ` (7 preceding siblings ...)
  2005-12-13 14:06 ` Prarit Bhargava
@ 2005-12-13 15:10 ` Andreas Schwab
  2005-12-13 15:42 ` Prarit Bhargava
                   ` (14 subsequent siblings)
  23 siblings, 0 replies; 25+ messages in thread
From: Andreas Schwab @ 2005-12-13 15:10 UTC (permalink / raw)
  To: linux-ia64

Prarit Bhargava <prarit@sgi.com> writes:

> Andreas -- I just recieved the following from a PPC engineer at IBM:
>
> "I don't think it is possible to build a ppc kernel with just
> CONFIG_PPC_MULTIPLATFORM set. You'll need to "choose" another compile
> option such as "cell" or "iseries" or "pseries" or somesuch (from
> arch/powerpc/platforms/). Check arch/powerpc/configs for the relevant
> defconfigs."
>
> ... so it looks like this is currently limited to ia64 ...

CONFIG_PPC_MULTIPLATFORM is just a meta config that enables you to
configure for multiple platforms.  In this way it is perfectly comparable
to CONFIG_IA64_GENERIC.  The only difference is that CONFIG_IA64_GENERIC
enables _all_ platforms with one option, whereas CONFIG_PPC_MULTIPLATFORM
still lets you chose which one of the platforms to support at all, but you
can chose to support more than one in a single kernel.

Andreas.

-- 
Andreas Schwab, SuSE Labs, schwab@suse.de
SuSE Linux Products GmbH, Maxfeldstraße 5, 90409 Nürnberg, Germany
PGP key fingerprint = 58CA 54C7 6D53 942B 1756  01D3 44D5 214B 8276 4ED5
"And now for something completely different."

^ permalink raw reply	[flat|nested] 25+ messages in thread

* Re: [PATCH]: Prevent sn2 ptc code from executing on all ia64 subarches
  2005-11-21 18:00 [PATCH]: Prevent sn2 ptc code from executing on all ia64 subarches Prarit Bhargava
                   ` (8 preceding siblings ...)
  2005-12-13 15:10 ` Andreas Schwab
@ 2005-12-13 15:42 ` Prarit Bhargava
  2005-12-13 15:43 ` Prarit Bhargava
                   ` (13 subsequent siblings)
  23 siblings, 0 replies; 25+ messages in thread
From: Prarit Bhargava @ 2005-12-13 15:42 UTC (permalink / raw)
  To: linux-ia64

This is sort of a mini-RFC for linux-ia64 -- just to see if anyone objects to 
the solution before I implement it.  I'll throw this out onto lkml after I get 
some input here.

I spent the past day or so looking at a couple of solutions to this problem and 
the only solution that appears viable is to (as stated previously) create new 
initcalls of the form

platform_calltype_initcall(fn,platform)

ex)
	platform_device_initcall(sn_prarit_driver, "sn2");

which would check the platform type prior to loading a kernel.

As it turns out this is a little bit more tricky than I thought as the initcalls 
are placed in a list of initcalls in the .initcall section.

do_initcalls then goes through this list in order to run the various initcalls 
on the list.

Since the macro calltype_initcall is placing the specified function in the 
.initcall list we cannot do

if (ia64_platform_is(foo))
	platform_calltype_initcall(fn,platform)

So I'm modifying my original suggestion.  We will still have

platform_calltype_initcall(fn,platform)

ex)

platform_device_initcall(sn_prarit_driver, IA64_PLATFORM_SN2);

but instead of doing the above wrapper, we would create a new section called 
.platform (name of the section is open to debate) and would traverse both the 
.initcall and the .platform sections at the sametime.

Therefore the could would be something like (this is based off the existing 
do_initcalls)

for (call = __initcall_start, platform = __platform_start;
      call < __initcall_end; call++, platform++) {


	if (platform = __platform_end)
		panic(); /* BUG? size(.platform) != size(!initcall) */

	if (platform && IA64_PLATFORM)
		(*call)();

where platform would be some bitmask that represents the various flavours of 
ia64 that the initcall can run on, and IA64_PLATFORM is set very early in the init.

A few other things:

- by default the value of platform would default to all platforms.

ie)
	device_initcall(sn_prarit_driver)

resolves to

	platform_device_initcall(sn_prarit_driver, IA64_PLATFORM_ALL);

- maybe there should be a runtime warning noting that you are using initcall or 
calltype_initcall on ia64?  (Someone convince me that this is really needed)

- I passed this by an ia64 engineer from HP and he made the comment that the 
nice thing about Linux is that the initcalls on all platforms are handled in 
EXACTLY the same manner.  I sort of agree with his assessment, however, no other 
  arch in Linux can compile a generic kernel and run on all of it's subarches...

Suggestions/comments?  Anyone think this is too much bother for the original 
issue of avoiding

	if (!ia64_platform_is("sn2"))
		return -ENOSYS;

?

P.

^ permalink raw reply	[flat|nested] 25+ messages in thread

* Re: [PATCH]: Prevent sn2 ptc code from executing on all ia64 subarches
  2005-11-21 18:00 [PATCH]: Prevent sn2 ptc code from executing on all ia64 subarches Prarit Bhargava
                   ` (9 preceding siblings ...)
  2005-12-13 15:42 ` Prarit Bhargava
@ 2005-12-13 15:43 ` Prarit Bhargava
  2005-12-13 16:03 ` Prarit Bhargava
                   ` (12 subsequent siblings)
  23 siblings, 0 replies; 25+ messages in thread
From: Prarit Bhargava @ 2005-12-13 15:43 UTC (permalink / raw)
  To: linux-ia64


> 
> CONFIG_PPC_MULTIPLATFORM is just a meta config that enables you to
> configure for multiple platforms.  In this way it is perfectly comparable
> to CONFIG_IA64_GENERIC.  The only difference is that CONFIG_IA64_GENERIC
> enables _all_ platforms with one option, whereas CONFIG_PPC_MULTIPLATFORM
> still lets you chose which one of the platforms to support at all, but you
> can chose to support more than one in a single kernel.

Oh .. thanks :)

P.

> 
> Andreas.
> 


^ permalink raw reply	[flat|nested] 25+ messages in thread

* Re: [PATCH]: Prevent sn2 ptc code from executing on all ia64 subarches
  2005-11-21 18:00 [PATCH]: Prevent sn2 ptc code from executing on all ia64 subarches Prarit Bhargava
                   ` (10 preceding siblings ...)
  2005-12-13 15:43 ` Prarit Bhargava
@ 2005-12-13 16:03 ` Prarit Bhargava
  2005-12-13 17:02 ` Christoph Hellwig
                   ` (11 subsequent siblings)
  23 siblings, 0 replies; 25+ messages in thread
From: Prarit Bhargava @ 2005-12-13 16:03 UTC (permalink / raw)
  To: linux-ia64

Andreas Schwab wrote:


 >I sort of agree with his assessment, however, no other
   arch in Linux can compile a generic kernel and run on all of it's subarches...

Kill that statement and replace with Andreas' note...

P.

^ permalink raw reply	[flat|nested] 25+ messages in thread

* Re: [PATCH]: Prevent sn2 ptc code from executing on all ia64 subarches
  2005-11-21 18:00 [PATCH]: Prevent sn2 ptc code from executing on all ia64 subarches Prarit Bhargava
                   ` (11 preceding siblings ...)
  2005-12-13 16:03 ` Prarit Bhargava
@ 2005-12-13 17:02 ` Christoph Hellwig
  2005-12-13 17:14 ` Bjorn Helgaas
                   ` (10 subsequent siblings)
  23 siblings, 0 replies; 25+ messages in thread
From: Christoph Hellwig @ 2005-12-13 17:02 UTC (permalink / raw)
  To: linux-ia64

On Tue, Dec 13, 2005 at 11:03:38AM -0500, Prarit Bhargava wrote:
> Andreas Schwab wrote:
> 
> 
> >I sort of agree with his assessment, however, no other
>   arch in Linux can compile a generic kernel and run on all of it's 
>   subarches...
> 
> Kill that statement and replace with Andreas' note...

Same is true for alpha aswell, btw.


^ permalink raw reply	[flat|nested] 25+ messages in thread

* Re: [PATCH]: Prevent sn2 ptc code from executing on all ia64 subarches
  2005-11-21 18:00 [PATCH]: Prevent sn2 ptc code from executing on all ia64 subarches Prarit Bhargava
                   ` (12 preceding siblings ...)
  2005-12-13 17:02 ` Christoph Hellwig
@ 2005-12-13 17:14 ` Bjorn Helgaas
  2005-12-13 17:24 ` Luck, Tony
                   ` (9 subsequent siblings)
  23 siblings, 0 replies; 25+ messages in thread
From: Bjorn Helgaas @ 2005-12-13 17:14 UTC (permalink / raw)
  To: linux-ia64

On Tuesday 13 December 2005 8:42 am, Prarit Bhargava wrote:
> I spent the past day or so looking at a couple of solutions to this problem and 
> the only solution that appears viable is to (as stated previously) create new 
> initcalls of the form
> 
> platform_calltype_initcall(fn,platform)
> 
> ex)
> 	platform_device_initcall(sn_prarit_driver, "sn2");
> 
> which would check the platform type prior to loading a kernel.

I think this is too complicated.  Initcall ordering is fragile as it
is.  I don't think there are enough users of this sort of functionality
to justify complicating it even more with a platform identifier.

> Suggestions/comments?  Anyone think this is too much bother for the original 
> issue of avoiding
> 
> 	if (!ia64_platform_is("sn2"))
> 		return -ENOSYS;

But I still think it is important to keep these ia64_platform_is()
checks out of generic init functions.  There's just too much chance
of inadvertently breaking non-SN platforms.

Can you use the existing machine vector infrastructure to solve this
problem?  For example, what if you added a "platform_device_setup()"
hook that is a no-op on most platforms, but maps to sn_device_setup()
in the SN2 machine vector?  platform_device_setup() would itself be
called as a device_initcall().

^ permalink raw reply	[flat|nested] 25+ messages in thread

* RE: [PATCH]: Prevent sn2 ptc code from executing on all ia64 subarches
  2005-11-21 18:00 [PATCH]: Prevent sn2 ptc code from executing on all ia64 subarches Prarit Bhargava
                   ` (13 preceding siblings ...)
  2005-12-13 17:14 ` Bjorn Helgaas
@ 2005-12-13 17:24 ` Luck, Tony
  2005-12-13 17:47 ` Bjorn Helgaas
                   ` (8 subsequent siblings)
  23 siblings, 0 replies; 25+ messages in thread
From: Luck, Tony @ 2005-12-13 17:24 UTC (permalink / raw)
  To: linux-ia64

> I think this is too complicated.  Initcall ordering is fragile as it
> is. 
...

> Can you use the existing machine vector infrastructure to solve this
> problem?  For example, what if you added a "platform_device_setup()"
> hook that is a no-op on most platforms, but maps to sn_device_setup()
> in the SN2 machine vector?  platform_device_setup() would itself be
> called as a device_initcall().

This solution would move all the sn2 initializaions into the
device_initcall sequence.  Which might cause some ordering
problems.  E.g. sn_pci_init() is a "subsys_initcall" ... and I
expect that it really needs to happen before any of the
device_initcalls.

So there would have to be a sn_* version for potentially each of
the 7 existing levels of initcall to keep the ordering.

-Tony

^ permalink raw reply	[flat|nested] 25+ messages in thread

* Re: [PATCH]: Prevent sn2 ptc code from executing on all ia64 subarches
  2005-11-21 18:00 [PATCH]: Prevent sn2 ptc code from executing on all ia64 subarches Prarit Bhargava
                   ` (14 preceding siblings ...)
  2005-12-13 17:24 ` Luck, Tony
@ 2005-12-13 17:47 ` Bjorn Helgaas
  2005-12-13 17:57 ` Luck, Tony
                   ` (7 subsequent siblings)
  23 siblings, 0 replies; 25+ messages in thread
From: Bjorn Helgaas @ 2005-12-13 17:47 UTC (permalink / raw)
  To: linux-ia64

On Tuesday 13 December 2005 10:24 am, Luck, Tony wrote:
> > I think this is too complicated.  Initcall ordering is fragile as it
> > is. 
> ...
> 
> > Can you use the existing machine vector infrastructure to solve this
> > problem?  For example, what if you added a "platform_device_setup()"
> > hook that is a no-op on most platforms, but maps to sn_device_setup()
> > in the SN2 machine vector?  platform_device_setup() would itself be
> > called as a device_initcall().
> 
> This solution would move all the sn2 initializaions into the
> device_initcall sequence.  Which might cause some ordering
> problems.  E.g. sn_pci_init() is a "subsys_initcall" ... and I
> expect that it really needs to happen before any of the
> device_initcalls.

Well, sure.  device_setup() was just an example.  I expect they'd
want to add more (subsys, etc).  I doubt that all the levels would
be needed, but even if they were, I'd prefer it over sprinkling
"ia64_platform_is" everywhere.

^ permalink raw reply	[flat|nested] 25+ messages in thread

* Re: [PATCH]: Prevent sn2 ptc code from executing on all ia64 subarches
  2005-11-21 18:00 [PATCH]: Prevent sn2 ptc code from executing on all ia64 subarches Prarit Bhargava
                   ` (15 preceding siblings ...)
  2005-12-13 17:47 ` Bjorn Helgaas
@ 2005-12-13 17:57 ` Luck, Tony
  2005-12-13 18:26 ` Prarit Bhargava
                   ` (6 subsequent siblings)
  23 siblings, 0 replies; 25+ messages in thread
From: Luck, Tony @ 2005-12-13 17:57 UTC (permalink / raw)
  To: linux-ia64

On Tue, Dec 13, 2005 at 10:42:28AM -0500, Prarit Bhargava wrote:
> Therefore the could would be something like (this is based off the existing 
> do_initcalls)
> 
> for (call = __initcall_start, platform = __platform_start;
>       call < __initcall_end; call++, platform++) {

Running two separate parallel structures seems complex.
Why not make the entries in the .initcall sections be tuples
instead of just being an array:

struct initcalls {
	initcall_t	call;
#ifdef	CONFIG_PLATFORM_SPECIFIC_INIT
	char		*platform;
#endif
};

#ifndef CONFIG_PLATFORM_SPECIFIC_INIT
#define	arch_match(str) 1
old definition of __define_initcall (modified for structure}
#else
new definition of __define_initcall initializes to "{ fn, 0 }"
plus a __define_initcall_platform() that takes the extra arg
and initializes to "{ fn, plat }"
#endif

Then the do_initcalls loop looks like this:

	struct initcalls *call;

	for (call = __initcall_start; call < __initcall_end; call++) {
		if (!arch_match(call->platform))
			continue;
		(*call->call)();


		...
	}

-Tony

^ permalink raw reply	[flat|nested] 25+ messages in thread

* Re: [PATCH]: Prevent sn2 ptc code from executing on all ia64 subarches
  2005-11-21 18:00 [PATCH]: Prevent sn2 ptc code from executing on all ia64 subarches Prarit Bhargava
                   ` (16 preceding siblings ...)
  2005-12-13 17:57 ` Luck, Tony
@ 2005-12-13 18:26 ` Prarit Bhargava
  2005-12-13 20:27 ` Bjorn Helgaas
                   ` (5 subsequent siblings)
  23 siblings, 0 replies; 25+ messages in thread
From: Prarit Bhargava @ 2005-12-13 18:26 UTC (permalink / raw)
  To: linux-ia64

Bjorn Helgaas wrote:

> 
> Well, sure.  device_setup() was just an example.  I expect they'd
> want to add more (subsys, etc).  I doubt that all the levels would
> be needed, but even if they were, I'd prefer it over sprinkling
> "ia64_platform_is" everywhere.
> 

OTOH the moment they change the initcall sequence we would
have to change our machine vector interfaces.  And AFAICT no
one is happy with the 7 levels of init (everything from too
granular to not granular enough).

I'm inclined to move the direction that Tony suggests:

Tony Luck wrote:
> Running two separate parallel structures seems complex.
> Why not make the entries in the .initcall sections be tuples
> instead of just being an array:
> 

I think this seems a lot more sane than what I suggested.

I'll code something up and try to see if I can dig up a PPC
system to test on as well.

P.

^ permalink raw reply	[flat|nested] 25+ messages in thread

* Re: [PATCH]: Prevent sn2 ptc code from executing on all ia64 subarches
  2005-11-21 18:00 [PATCH]: Prevent sn2 ptc code from executing on all ia64 subarches Prarit Bhargava
                   ` (17 preceding siblings ...)
  2005-12-13 18:26 ` Prarit Bhargava
@ 2005-12-13 20:27 ` Bjorn Helgaas
  2005-12-14 13:17 ` Prarit Bhargava
                   ` (4 subsequent siblings)
  23 siblings, 0 replies; 25+ messages in thread
From: Bjorn Helgaas @ 2005-12-13 20:27 UTC (permalink / raw)
  To: linux-ia64

On Tuesday 13 December 2005 11:26 am, Prarit Bhargava wrote:
> OTOH the moment they change the initcall sequence we would
> have to change our machine vector interfaces.  And AFAICT no
> one is happy with the 7 levels of init (everything from too
> granular to not granular enough).

Whoa, hold on a minute.  Let's back up.

Most of the uses of ia64_platform_is() are really just hacks
to bind drivers to devices that only exist on SN2:

  arch/ia64/sn/kernel/tiocx.c          tiocx_init()
  arch/ia64/sn/kernel/xp_main.c        xp_init()
  arch/ia64/sn/kernel/xpc_main.c       xpc_init()
  arch/ia64/sn/kernel/xpnet.c          xpnet_init()
  arch/ia64/sn/kernel/sn2/sn_hwperf.c  sn_hwperf_misc_register_init()
  drivers/char/mbcs.c                  mbcs_init()
  drivers/char/mmtimer.c               mmtimer_init()
  drivers/char/snsc.c                  scdrv_init()
  drivers/pci/hotplug/sgi_hotplug.c    sn_pci_hotplug_init()
  drivers/serial/sn_console.c          sn_sal_module_init()
  drivers/sn/ioc4.c                    ioc4_init()

It's totally backwards to limit driver binding by using
ia64_platform_is().

You ought to just describe this hardware in the ACPI namespace
and use acpi_bus_register_driver() to bind the drivers.

Then you can register the drivers on all platforms, but the .add()
function (and hence, the rest of the driver) will only be called
when the hardware is actually present.  So you don't need any
platform-qualified initcalls.

^ permalink raw reply	[flat|nested] 25+ messages in thread

* Re: [PATCH]: Prevent sn2 ptc code from executing on all ia64 subarches
  2005-11-21 18:00 [PATCH]: Prevent sn2 ptc code from executing on all ia64 subarches Prarit Bhargava
                   ` (18 preceding siblings ...)
  2005-12-13 20:27 ` Bjorn Helgaas
@ 2005-12-14 13:17 ` Prarit Bhargava
  2005-12-15 14:33 ` Prarit Bhargava
                   ` (3 subsequent siblings)
  23 siblings, 0 replies; 25+ messages in thread
From: Prarit Bhargava @ 2005-12-14 13:17 UTC (permalink / raw)
  To: linux-ia64

Bjorn Helgaas wrote:

> 
> Then you can register the drivers on all platforms, but the .add()
> function (and hence, the rest of the driver) will only be called
> when the hardware is actually present.  So you don't need any
> platform-qualified initcalls.
> 

Hmmm ... I haven't done alot with ACPI but I'll go take a look.

This seems to handle the drivers ... but the issue of the other
initcalls remains the same?  What do I do with layers above/below
the device calls (note, that some of the calls you list above are
NOT device_initcalls, but subsys_initcalls in order to get the
driver load order correct)?  What about non-device level
initcalls like sn_pci_init?

I suppose I could in that once instance use ia64_platform_is.
But then we're in the same boat we were in earlier ...

P.


^ permalink raw reply	[flat|nested] 25+ messages in thread

* Re: [PATCH]: Prevent sn2 ptc code from executing on all ia64 subarches
  2005-11-21 18:00 [PATCH]: Prevent sn2 ptc code from executing on all ia64 subarches Prarit Bhargava
                   ` (19 preceding siblings ...)
  2005-12-14 13:17 ` Prarit Bhargava
@ 2005-12-15 14:33 ` Prarit Bhargava
  2005-12-15 15:51 ` Bjorn Helgaas
                   ` (2 subsequent siblings)
  23 siblings, 0 replies; 25+ messages in thread
From: Prarit Bhargava @ 2005-12-15 14:33 UTC (permalink / raw)
  To: linux-ia64

Bjorn Helgaas wrote:

> 
> You ought to just describe this hardware in the ACPI namespace
> and use acpi_bus_register_driver() to bind the drivers.

ACPI support is in the process of being implemented.
Currently, there is no ACPI DSDT table being created on
SN systems.

P.

^ permalink raw reply	[flat|nested] 25+ messages in thread

* Re: [PATCH]: Prevent sn2 ptc code from executing on all ia64 subarches
  2005-11-21 18:00 [PATCH]: Prevent sn2 ptc code from executing on all ia64 subarches Prarit Bhargava
                   ` (20 preceding siblings ...)
  2005-12-15 14:33 ` Prarit Bhargava
@ 2005-12-15 15:51 ` Bjorn Helgaas
  2005-12-15 20:56 ` Prarit Bhargava
  2005-12-15 21:08 ` Luck, Tony
  23 siblings, 0 replies; 25+ messages in thread
From: Bjorn Helgaas @ 2005-12-15 15:51 UTC (permalink / raw)
  To: linux-ia64

On Thursday 15 December 2005 7:33 am, Prarit Bhargava wrote:
> Bjorn Helgaas wrote:
> > You ought to just describe this hardware in the ACPI namespace
> > and use acpi_bus_register_driver() to bind the drivers.
> 
> ACPI support is in the process of being implemented.
> Currently, there is no ACPI DSDT table being created on
> SN systems.

You ought to be able to compile an SSDT into the kernel and
load it at boot-time, e.g., by extending acpi_os_table_override().
Or you might do something like arch/parisc/kernel/drivers.c.
Or maybe there's a way to use platform_devices.

The point is, I think it's a mistake to use ia64_platform_is()
to decide whether to bind a driver to a device.  I think it would
be much better to make it explicit that these are drivers, and
they only bind to specific devices.

I do recognize that this doesn't cover all the current uses of
ia64_platform_is(), but it covers a pretty big chunk of them.

^ permalink raw reply	[flat|nested] 25+ messages in thread

* Re: [PATCH]: Prevent sn2 ptc code from executing on all ia64 subarches
  2005-11-21 18:00 [PATCH]: Prevent sn2 ptc code from executing on all ia64 subarches Prarit Bhargava
                   ` (21 preceding siblings ...)
  2005-12-15 15:51 ` Bjorn Helgaas
@ 2005-12-15 20:56 ` Prarit Bhargava
  2005-12-15 21:08 ` Luck, Tony
  23 siblings, 0 replies; 25+ messages in thread
From: Prarit Bhargava @ 2005-12-15 20:56 UTC (permalink / raw)
  To: linux-ia64

Bjorn Helgaas wrote:

> 
> The point is, I think it's a mistake to use ia64_platform_is()
> to decide whether to bind a driver to a device.  I think it would
> be much better to make it explicit that these are drivers, and
> they only bind to specific devices.
> 

Yes, I agree with that ...

> I do recognize that this doesn't cover all the current uses of
> ia64_platform_is(), but it covers a pretty big chunk of them.
> 

Which prolly means that I should continue with the solution that was 
previously put forward.

P.

^ permalink raw reply	[flat|nested] 25+ messages in thread

* RE: [PATCH]: Prevent sn2 ptc code from executing on all ia64 subarches
  2005-11-21 18:00 [PATCH]: Prevent sn2 ptc code from executing on all ia64 subarches Prarit Bhargava
                   ` (22 preceding siblings ...)
  2005-12-15 20:56 ` Prarit Bhargava
@ 2005-12-15 21:08 ` Luck, Tony
  23 siblings, 0 replies; 25+ messages in thread
From: Luck, Tony @ 2005-12-15 21:08 UTC (permalink / raw)
  To: linux-ia64

>> I do recognize that this doesn't cover all the current uses of
>> ia64_platform_is(), but it covers a pretty big chunk of them.
>> 
>
>Which prolly means that I should continue with the solution that was 
>previously put forward.

Not necessarily.  If all the device uses can be fixed with a fake
ACPI table, and if the remaining cases all fall into just a couple
of the six other init levels, then it might be sufficent to just make
sn_init functions for the two levels that are used.

You may need this as a backup plan anyway, the "tuples in the init
table" only has a limited chance of success of getting accepted into
mainline.  Depends on how pretty it comes out, and whether anyone
other than ia64 will actually use it.

-Tony 

^ permalink raw reply	[flat|nested] 25+ messages in thread

end of thread, other threads:[~2005-12-15 21:08 UTC | newest]

Thread overview: 25+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2005-11-21 18:00 [PATCH]: Prevent sn2 ptc code from executing on all ia64 subarches Prarit Bhargava
2005-12-09 17:11 ` Robin Holt
2005-12-09 17:46 ` Bjorn Helgaas
2005-12-09 17:54 ` Luck, Tony
2005-12-09 18:22 ` Prarit Bhargava
2005-12-09 18:31 ` Prarit Bhargava
2005-12-09 18:38 ` Luck, Tony
2005-12-09 19:41 ` Andreas Schwab
2005-12-13 14:06 ` Prarit Bhargava
2005-12-13 15:10 ` Andreas Schwab
2005-12-13 15:42 ` Prarit Bhargava
2005-12-13 15:43 ` Prarit Bhargava
2005-12-13 16:03 ` Prarit Bhargava
2005-12-13 17:02 ` Christoph Hellwig
2005-12-13 17:14 ` Bjorn Helgaas
2005-12-13 17:24 ` Luck, Tony
2005-12-13 17:47 ` Bjorn Helgaas
2005-12-13 17:57 ` Luck, Tony
2005-12-13 18:26 ` Prarit Bhargava
2005-12-13 20:27 ` Bjorn Helgaas
2005-12-14 13:17 ` Prarit Bhargava
2005-12-15 14:33 ` Prarit Bhargava
2005-12-15 15:51 ` Bjorn Helgaas
2005-12-15 20:56 ` Prarit Bhargava
2005-12-15 21:08 ` Luck, Tony

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.