Re: [PATCH v5 3/3] xen/arm: introduce xen_early_init, use PSCI on xen

2013-04-03 Thread Stefano Stabellini
On Wed, 3 Apr 2013, Nicolas Pitre wrote:
> On Wed, 3 Apr 2013, Stefano Stabellini wrote:
> 
> > On Tue, 2 Apr 2013, Nicolas Pitre wrote:
> > > On Tue, 2 Apr 2013, Stefano Stabellini wrote:
> > > 
> > > > @@ -176,27 +178,30 @@ static int __init xen_secondary_init(unsigned int 
> > > > cpu)
> > > > return 0;
> > > >  }
> > > >  
> > > > +static void __init xen_smp_init(void)
> > > > +{
> > > > +   if (psci_smp_available())
> > > > +   smp_set_ops(_smp_ops);
> > > > +}
> > > > +
> > > 
> > > I still don't understand why you need to do this.  Why can't you just 
> > > rely on the next priority which is to set PSCI ops by default if 
> > > available?  Using this hook for Xen doesn't look justified. As it is, 
> > > you break MCPM.
> > > 
> > > As I explained to you, MCPM will end up using PSCI as well when 
> > > available, which I think should be sufficient for your concern.
> > 
> > The smp_init hook is not limited to MCPM or the versatile express
> > platform. It's a generic hook that can be used by any platform and can
> > override the platform smp_ops or the psci_smp_ops depending on platform
> > specific configurations.
> > 
> > Configurations that I am pretty sure won't be available on Xen anyway,
> > while I am certain that using psci_smp_ops would work.
> > 
> > It seems to me that relying on the fact that only versatile express and
> > only MCPM use smp_init, even though it might work today, it's very
> > fragile and could break tomorrow without any of us noticing.
> 
> I claim: this breaks MCPM today.
>
> You claim: the alternative _could_ break with Xen tomorrow.
> 
> Could we please wait until there is an actual problem with Xen before 
> being overly defensive in the code?

Sorry, I realized that I should have explained myself better.

The smp_init patch together with the MCPM patch series
(http://marc.info/?l=linux-arm-kernel=136004188122492) breaks Xen Dom0
SMP support on versatile express *today* (ifndef CONFIG_CLUSTER_PM):
smp_init is not NULL on versatile express anymore therefore smp_init is
going to be called, causing vexpress_smp_ops to be selected even if psci
is on device tree.

On the other hand if this patch was applied, xen_smp_init would override
smp_init making sure that psci_smp_ops is used on Xen and everything
would work as expected.


So as it stands today, we need this patch for regular Xen Dom0 SMP
support.


BTW I have actually tried it on my versatile express machine to make
sure that it is correct :)
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/


Re: [PATCH v5 3/3] xen/arm: introduce xen_early_init, use PSCI on xen

2013-04-03 Thread Nicolas Pitre
On Wed, 3 Apr 2013, Stefano Stabellini wrote:

> On Tue, 2 Apr 2013, Nicolas Pitre wrote:
> > On Tue, 2 Apr 2013, Stefano Stabellini wrote:
> > 
> > > @@ -176,27 +178,30 @@ static int __init xen_secondary_init(unsigned int 
> > > cpu)
> > >   return 0;
> > >  }
> > >  
> > > +static void __init xen_smp_init(void)
> > > +{
> > > + if (psci_smp_available())
> > > + smp_set_ops(_smp_ops);
> > > +}
> > > +
> > 
> > I still don't understand why you need to do this.  Why can't you just 
> > rely on the next priority which is to set PSCI ops by default if 
> > available?  Using this hook for Xen doesn't look justified. As it is, 
> > you break MCPM.
> > 
> > As I explained to you, MCPM will end up using PSCI as well when 
> > available, which I think should be sufficient for your concern.
> 
> The smp_init hook is not limited to MCPM or the versatile express
> platform. It's a generic hook that can be used by any platform and can
> override the platform smp_ops or the psci_smp_ops depending on platform
> specific configurations.
> 
> Configurations that I am pretty sure won't be available on Xen anyway,
> while I am certain that using psci_smp_ops would work.
> 
> It seems to me that relying on the fact that only versatile express and
> only MCPM use smp_init, even though it might work today, it's very
> fragile and could break tomorrow without any of us noticing.

I claim: this breaks MCPM today.

You claim: the alternative _could_ break with Xen tomorrow.

Could we please wait until there is an actual problem with Xen before 
being overly defensive in the code?


Nicolas
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/


Re: [PATCH v5 3/3] xen/arm: introduce xen_early_init, use PSCI on xen

2013-04-03 Thread Stefano Stabellini
On Tue, 2 Apr 2013, Nicolas Pitre wrote:
> On Tue, 2 Apr 2013, Stefano Stabellini wrote:
> 
> > Split xen_guest_init in two functions, one of them (xen_early_init) is
> > going to be called very early from setup_arch.
> > 
> > Change machine_desc->smp_init to xen_smp_init if Xen is present on the
> > platform. xen_smp_init just sets smp_ops to psci_smp_ops.
> > 
> > Introduce a dependency for ARM_PSCI in XEN.
> 
> The Kconfig stuff should be more understandable to "normal" users 
> configuring the kernel.  Hence it might make more sense for Xen to 
> select PSCI rather than making it a prerequisite.

You are right, I'll do that.


> [...]
> > @@ -176,27 +178,30 @@ static int __init xen_secondary_init(unsigned int cpu)
> > return 0;
> >  }
> >  
> > +static void __init xen_smp_init(void)
> > +{
> > +   if (psci_smp_available())
> > +   smp_set_ops(_smp_ops);
> > +}
> > +
> 
> I still don't understand why you need to do this.  Why can't you just 
> rely on the next priority which is to set PSCI ops by default if 
> available?  Using this hook for Xen doesn't look justified. As it is, 
> you break MCPM.
> 
> As I explained to you, MCPM will end up using PSCI as well when 
> available, which I think should be sufficient for your concern.

The smp_init hook is not limited to MCPM or the versatile express
platform. It's a generic hook that can be used by any platform and can
override the platform smp_ops or the psci_smp_ops depending on platform
specific configurations.

Configurations that I am pretty sure won't be available on Xen anyway,
while I am certain that using psci_smp_ops would work.

It seems to me that relying on the fact that only versatile express and
only MCPM use smp_init, even though it might work today, it's very
fragile and could break tomorrow without any of us noticing.
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/


Re: [PATCH v5 3/3] xen/arm: introduce xen_early_init, use PSCI on xen

2013-04-03 Thread Stefano Stabellini
On Tue, 2 Apr 2013, Nicolas Pitre wrote:
 On Tue, 2 Apr 2013, Stefano Stabellini wrote:
 
  Split xen_guest_init in two functions, one of them (xen_early_init) is
  going to be called very early from setup_arch.
  
  Change machine_desc-smp_init to xen_smp_init if Xen is present on the
  platform. xen_smp_init just sets smp_ops to psci_smp_ops.
  
  Introduce a dependency for ARM_PSCI in XEN.
 
 The Kconfig stuff should be more understandable to normal users 
 configuring the kernel.  Hence it might make more sense for Xen to 
 select PSCI rather than making it a prerequisite.

You are right, I'll do that.


 [...]
  @@ -176,27 +178,30 @@ static int __init xen_secondary_init(unsigned int cpu)
  return 0;
   }
   
  +static void __init xen_smp_init(void)
  +{
  +   if (psci_smp_available())
  +   smp_set_ops(psci_smp_ops);
  +}
  +
 
 I still don't understand why you need to do this.  Why can't you just 
 rely on the next priority which is to set PSCI ops by default if 
 available?  Using this hook for Xen doesn't look justified. As it is, 
 you break MCPM.
 
 As I explained to you, MCPM will end up using PSCI as well when 
 available, which I think should be sufficient for your concern.

The smp_init hook is not limited to MCPM or the versatile express
platform. It's a generic hook that can be used by any platform and can
override the platform smp_ops or the psci_smp_ops depending on platform
specific configurations.

Configurations that I am pretty sure won't be available on Xen anyway,
while I am certain that using psci_smp_ops would work.

It seems to me that relying on the fact that only versatile express and
only MCPM use smp_init, even though it might work today, it's very
fragile and could break tomorrow without any of us noticing.
--
To unsubscribe from this list: send the line unsubscribe linux-kernel in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/


Re: [PATCH v5 3/3] xen/arm: introduce xen_early_init, use PSCI on xen

2013-04-03 Thread Nicolas Pitre
On Wed, 3 Apr 2013, Stefano Stabellini wrote:

 On Tue, 2 Apr 2013, Nicolas Pitre wrote:
  On Tue, 2 Apr 2013, Stefano Stabellini wrote:
  
   @@ -176,27 +178,30 @@ static int __init xen_secondary_init(unsigned int 
   cpu)
 return 0;
}

   +static void __init xen_smp_init(void)
   +{
   + if (psci_smp_available())
   + smp_set_ops(psci_smp_ops);
   +}
   +
  
  I still don't understand why you need to do this.  Why can't you just 
  rely on the next priority which is to set PSCI ops by default if 
  available?  Using this hook for Xen doesn't look justified. As it is, 
  you break MCPM.
  
  As I explained to you, MCPM will end up using PSCI as well when 
  available, which I think should be sufficient for your concern.
 
 The smp_init hook is not limited to MCPM or the versatile express
 platform. It's a generic hook that can be used by any platform and can
 override the platform smp_ops or the psci_smp_ops depending on platform
 specific configurations.
 
 Configurations that I am pretty sure won't be available on Xen anyway,
 while I am certain that using psci_smp_ops would work.
 
 It seems to me that relying on the fact that only versatile express and
 only MCPM use smp_init, even though it might work today, it's very
 fragile and could break tomorrow without any of us noticing.

I claim: this breaks MCPM today.

You claim: the alternative _could_ break with Xen tomorrow.

Could we please wait until there is an actual problem with Xen before 
being overly defensive in the code?


Nicolas
--
To unsubscribe from this list: send the line unsubscribe linux-kernel in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/


Re: [PATCH v5 3/3] xen/arm: introduce xen_early_init, use PSCI on xen

2013-04-03 Thread Stefano Stabellini
On Wed, 3 Apr 2013, Nicolas Pitre wrote:
 On Wed, 3 Apr 2013, Stefano Stabellini wrote:
 
  On Tue, 2 Apr 2013, Nicolas Pitre wrote:
   On Tue, 2 Apr 2013, Stefano Stabellini wrote:
   
@@ -176,27 +178,30 @@ static int __init xen_secondary_init(unsigned int 
cpu)
return 0;
 }
 
+static void __init xen_smp_init(void)
+{
+   if (psci_smp_available())
+   smp_set_ops(psci_smp_ops);
+}
+
   
   I still don't understand why you need to do this.  Why can't you just 
   rely on the next priority which is to set PSCI ops by default if 
   available?  Using this hook for Xen doesn't look justified. As it is, 
   you break MCPM.
   
   As I explained to you, MCPM will end up using PSCI as well when 
   available, which I think should be sufficient for your concern.
  
  The smp_init hook is not limited to MCPM or the versatile express
  platform. It's a generic hook that can be used by any platform and can
  override the platform smp_ops or the psci_smp_ops depending on platform
  specific configurations.
  
  Configurations that I am pretty sure won't be available on Xen anyway,
  while I am certain that using psci_smp_ops would work.
  
  It seems to me that relying on the fact that only versatile express and
  only MCPM use smp_init, even though it might work today, it's very
  fragile and could break tomorrow without any of us noticing.
 
 I claim: this breaks MCPM today.

 You claim: the alternative _could_ break with Xen tomorrow.
 
 Could we please wait until there is an actual problem with Xen before 
 being overly defensive in the code?

Sorry, I realized that I should have explained myself better.

The smp_init patch together with the MCPM patch series
(http://marc.info/?l=linux-arm-kernelm=136004188122492) breaks Xen Dom0
SMP support on versatile express *today* (ifndef CONFIG_CLUSTER_PM):
smp_init is not NULL on versatile express anymore therefore smp_init is
going to be called, causing vexpress_smp_ops to be selected even if psci
is on device tree.

On the other hand if this patch was applied, xen_smp_init would override
smp_init making sure that psci_smp_ops is used on Xen and everything
would work as expected.


So as it stands today, we need this patch for regular Xen Dom0 SMP
support.


BTW I have actually tried it on my versatile express machine to make
sure that it is correct :)
--
To unsubscribe from this list: send the line unsubscribe linux-kernel in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/


Re: [PATCH v5 3/3] xen/arm: introduce xen_early_init, use PSCI on xen

2013-04-02 Thread Nicolas Pitre
On Tue, 2 Apr 2013, Stefano Stabellini wrote:

> Split xen_guest_init in two functions, one of them (xen_early_init) is
> going to be called very early from setup_arch.
> 
> Change machine_desc->smp_init to xen_smp_init if Xen is present on the
> platform. xen_smp_init just sets smp_ops to psci_smp_ops.
> 
> Introduce a dependency for ARM_PSCI in XEN.

The Kconfig stuff should be more understandable to "normal" users 
configuring the kernel.  Hence it might make more sense for Xen to 
select PSCI rather than making it a prerequisite.

[...]
> @@ -176,27 +178,30 @@ static int __init xen_secondary_init(unsigned int cpu)
>   return 0;
>  }
>  
> +static void __init xen_smp_init(void)
> +{
> + if (psci_smp_available())
> + smp_set_ops(_smp_ops);
> +}
> +

I still don't understand why you need to do this.  Why can't you just 
rely on the next priority which is to set PSCI ops by default if 
available?  Using this hook for Xen doesn't look justified. As it is, 
you break MCPM.

As I explained to you, MCPM will end up using PSCI as well when 
available, which I think should be sufficient for your concern.


Nicolas
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/


Re: [PATCH v5 3/3] xen/arm: introduce xen_early_init, use PSCI on xen

2013-04-02 Thread Nicolas Pitre
On Tue, 2 Apr 2013, Stefano Stabellini wrote:

 Split xen_guest_init in two functions, one of them (xen_early_init) is
 going to be called very early from setup_arch.
 
 Change machine_desc-smp_init to xen_smp_init if Xen is present on the
 platform. xen_smp_init just sets smp_ops to psci_smp_ops.
 
 Introduce a dependency for ARM_PSCI in XEN.

The Kconfig stuff should be more understandable to normal users 
configuring the kernel.  Hence it might make more sense for Xen to 
select PSCI rather than making it a prerequisite.

[...]
 @@ -176,27 +178,30 @@ static int __init xen_secondary_init(unsigned int cpu)
   return 0;
  }
  
 +static void __init xen_smp_init(void)
 +{
 + if (psci_smp_available())
 + smp_set_ops(psci_smp_ops);
 +}
 +

I still don't understand why you need to do this.  Why can't you just 
rely on the next priority which is to set PSCI ops by default if 
available?  Using this hook for Xen doesn't look justified. As it is, 
you break MCPM.

As I explained to you, MCPM will end up using PSCI as well when 
available, which I think should be sufficient for your concern.


Nicolas
--
To unsubscribe from this list: send the line unsubscribe linux-kernel in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/