diff mbox

ARM: nommu: re-enable use of vexpress without ARCH_MULTIPLATFORM

Message ID 1357755328-17075-1-git-send-email-jonathan.austin@arm.com (mailing list archive)
State New, archived
Headers show

Commit Message

Jonathan Austin Jan. 9, 2013, 6:15 p.m. UTC
From: Will Deacon <will.deacon@arm.com>

Since 617276307cd4c ("ARM: vexpress: convert to multi-platform") it has
been impossible to select ARCH_VEXPRESS without ARCH_MULTIPLATFORM.

ARCH_MULTIPLATFORM doesn't make sense for NOMMU targets, not least
because of the need to hard-code the memory map. However, it should
still be possible to run NOMMU kernels on top of the Versatile Express
by selecting it as the only platform.

This patch creates a shim ARCH_VEXPRESS_NOMMU config option in the 'choice'
for "ARM system type" to make this possible again.

Signed-off-by: Will Deacon <will.deacon@arm.com>
Signed-off-by: Jonathan Austin <jonathan.austin@arm.com>
CC: Nicolas Pitre <nico@fluxnic.net>
CC: Arnd Bergmann <arnd@arndb.de>
---

Arnd and Nicolas: I've removed your acks as I've changed the location
of the new block (as Nicolas suggested) and ever so slightly tweaked
the name of the new option.

 arch/arm/Kconfig |    5 +++++
 1 file changed, 5 insertions(+)

Comments

Nicolas Pitre Jan. 9, 2013, 6:30 p.m. UTC | #1
On Wed, 9 Jan 2013, Jonathan Austin wrote:

> From: Will Deacon <will.deacon@arm.com>

Did Will actually write the patch?  It rather looks like you are the one 
folloing through, in which case you deserve the credits.

> Since 617276307cd4c ("ARM: vexpress: convert to multi-platform") it has
> been impossible to select ARCH_VEXPRESS without ARCH_MULTIPLATFORM.
> 
> ARCH_MULTIPLATFORM doesn't make sense for NOMMU targets, not least
> because of the need to hard-code the memory map. However, it should
> still be possible to run NOMMU kernels on top of the Versatile Express
> by selecting it as the only platform.
> 
> This patch creates a shim ARCH_VEXPRESS_NOMMU config option in the 'choice'
> for "ARM system type" to make this possible again.
> 
> Signed-off-by: Will Deacon <will.deacon@arm.com>
> Signed-off-by: Jonathan Austin <jonathan.austin@arm.com>
> CC: Nicolas Pitre <nico@fluxnic.net>
> CC: Arnd Bergmann <arnd@arndb.de>
> ---
> 
> Arnd and Nicolas: I've removed your acks as I've changed the location
> of the new block (as Nicolas suggested) and ever so slightly tweaked
> the name of the new option.

Please add some help text to explain what this is for as well.

And you may add my ACK back.



> 
>  arch/arm/Kconfig |    5 +++++
>  1 file changed, 5 insertions(+)
> 
> diff --git a/arch/arm/Kconfig b/arch/arm/Kconfig
> index 335e220..3357d0b 100644
> --- a/arch/arm/Kconfig
> +++ b/arch/arm/Kconfig
> @@ -322,6 +322,11 @@ config ARCH_VERSATILE
>  	help
>  	  This enables support for ARM Ltd Versatile board.
>  
> +config ARCH_VEXPRESS_NOMMU
> +	bool "ARM Ltd. Versatile Express family for NOMMU"
> +	depends on !MMU
> +	select ARCH_VEXPRESS
> +
>  config ARCH_AT91
>  	bool "Atmel AT91"
>  	select ARCH_REQUIRE_GPIOLIB
> -- 
> 1.7.9.5
> 
>
Will Deacon Jan. 9, 2013, 6:41 p.m. UTC | #2
On Wed, Jan 09, 2013 at 06:30:01PM +0000, Nicolas Pitre wrote:
> On Wed, 9 Jan 2013, Jonathan Austin wrote:
> 
> > From: Will Deacon <will.deacon@arm.com>
> 
> Did Will actually write the patch?  It rather looks like you are the one 
> folloing through, in which case you deserve the credits.

I wrote the initial patch, but it's pretty simple and I just want it fixed,
so I'm not especially bothered about the attribution. If you do remove me as
the author, you can leave my S-o-B after yours.

Cheers,

Will
Arnd Bergmann Jan. 9, 2013, 6:43 p.m. UTC | #3
On Wednesday 09 January 2013, Jonathan Austin wrote:
> From: Will Deacon <will.deacon@arm.com>
> 
> Since 617276307cd4c ("ARM: vexpress: convert to multi-platform") it has
> been impossible to select ARCH_VEXPRESS without ARCH_MULTIPLATFORM.
> 
> ARCH_MULTIPLATFORM doesn't make sense for NOMMU targets, not least
> because of the need to hard-code the memory map. However, it should
> still be possible to run NOMMU kernels on top of the Versatile Express
> by selecting it as the only platform.
> 
> This patch creates a shim ARCH_VEXPRESS_NOMMU config option in the 'choice'
> for "ARM system type" to make this possible again.
> 
> Signed-off-by: Will Deacon <will.deacon@arm.com>
> Signed-off-by: Jonathan Austin <jonathan.austin@arm.com>
> CC: Nicolas Pitre <nico@fluxnic.net>
> CC: Arnd Bergmann <arnd@arndb.de>
> ---
> 
> Arnd and Nicolas: I've removed your acks as I've changed the location
> of the new block (as Nicolas suggested) and ever so slightly tweaked
> the name of the new option.

The patch is still good. Generally you can leave an Ack when doing
small changes but leaving the patch conceptually the same.

Acked-by: Arnd Bergmann <arnd@arndb.de>

On a related topic, I still think we should fix ARCH_MULTI_V7 not
to select ARCH_VEXPRESS unconditionally and come up with a better
way to avoid having an empty platform list to make 'allnoconfig'
still work.

	Arnd
Nicolas Pitre Jan. 9, 2013, 6:54 p.m. UTC | #4
On Wed, 9 Jan 2013, Arnd Bergmann wrote:

> On a related topic, I still think we should fix ARCH_MULTI_V7 not
> to select ARCH_VEXPRESS unconditionally and come up with a better
> way to avoid having an empty platform list to make 'allnoconfig'
> still work.

The virtual guest platform support that Will and Marc did is small 
enough that it could always be selected in place of vexpress.


Nicolas
Arnd Bergmann Jan. 9, 2013, 8:22 p.m. UTC | #5
On Wednesday 09 January 2013, Nicolas Pitre wrote:
> On Wed, 9 Jan 2013, Arnd Bergmann wrote:
> 
> > On a related topic, I still think we should fix ARCH_MULTI_V7 not
> > to select ARCH_VEXPRESS unconditionally and come up with a better
> > way to avoid having an empty platform list to make 'allnoconfig'
> > still work.
> 
> The virtual guest platform support that Will and Marc did is small 
> enough that it could always be selected in place of vexpress.

But that only helps when ARMv7 is selected, unless we want to build
it only for ARMv4, v5 or v6 kernels.

Besides, the only reason we can't have a kernel without any platform
selected is that the linker script has code in it to intentionally
barf on that because it's guaranteed not to boot on any hardware.

If we decide that building an allnoconfig without any platform
is actually ok, we could just as well rip out that error statement.

	Arnd
Rob Herring Jan. 9, 2013, 8:39 p.m. UTC | #6
On 01/09/2013 02:22 PM, Arnd Bergmann wrote:
> On Wednesday 09 January 2013, Nicolas Pitre wrote:
>> On Wed, 9 Jan 2013, Arnd Bergmann wrote:
>>
>>> On a related topic, I still think we should fix ARCH_MULTI_V7 not
>>> to select ARCH_VEXPRESS unconditionally and come up with a better
>>> way to avoid having an empty platform list to make 'allnoconfig'
>>> still work.
>>
>> The virtual guest platform support that Will and Marc did is small 
>> enough that it could always be selected in place of vexpress.
> 
> But that only helps when ARMv7 is selected, unless we want to build
> it only for ARMv4, v5 or v6 kernels.
> 
> Besides, the only reason we can't have a kernel without any platform
> selected is that the linker script has code in it to intentionally
> barf on that because it's guaranteed not to boot on any hardware.
> 
> If we decide that building an allnoconfig without any platform
> is actually ok, we could just as well rip out that error statement.
> 

That patch is already posted, but Russell doesn't like it as you can
have a kernel that doesn't boot. You don't like the allno and randconfig
failures, so we're stuck. I think there are dozens of config options
that will make you not boot on any given platform, so failing to boot
because you did not select your machine is a non-issue.

Rob
Russell King - ARM Linux Jan. 9, 2013, 8:48 p.m. UTC | #7
On Wed, Jan 09, 2013 at 02:39:36PM -0600, Rob Herring wrote:
> On 01/09/2013 02:22 PM, Arnd Bergmann wrote:
> > On Wednesday 09 January 2013, Nicolas Pitre wrote:
> >> On Wed, 9 Jan 2013, Arnd Bergmann wrote:
> >>
> >>> On a related topic, I still think we should fix ARCH_MULTI_V7 not
> >>> to select ARCH_VEXPRESS unconditionally and come up with a better
> >>> way to avoid having an empty platform list to make 'allnoconfig'
> >>> still work.
> >>
> >> The virtual guest platform support that Will and Marc did is small 
> >> enough that it could always be selected in place of vexpress.
> > 
> > But that only helps when ARMv7 is selected, unless we want to build
> > it only for ARMv4, v5 or v6 kernels.
> > 
> > Besides, the only reason we can't have a kernel without any platform
> > selected is that the linker script has code in it to intentionally
> > barf on that because it's guaranteed not to boot on any hardware.
> > 
> > If we decide that building an allnoconfig without any platform
> > is actually ok, we could just as well rip out that error statement.
> > 
> 
> That patch is already posted, but Russell doesn't like it as you can
> have a kernel that doesn't boot. You don't like the allno and randconfig
> failures, so we're stuck. I think there are dozens of config options
> that will make you not boot on any given platform, so failing to boot
> because you did not select your machine is a non-issue.

What I actually suggested is that we should be aiming for the DT side
of things to get to the point where DT is just another _single_ platform
as far as that code goes, and that DT should describe the hardware
sufficiently well that we don't have multiple machine_desc things to
select via DT - so a DT kernel would have exactly one machine_desc (or
maybe even zero! - with the linker script check conditional on !CONFIG_OF)

That then gets rid of the issue entirely.
Nicolas Pitre Jan. 9, 2013, 9 p.m. UTC | #8
On Wed, 9 Jan 2013, Arnd Bergmann wrote:

> On Wednesday 09 January 2013, Nicolas Pitre wrote:
> > On Wed, 9 Jan 2013, Arnd Bergmann wrote:
> > 
> > > On a related topic, I still think we should fix ARCH_MULTI_V7 not
> > > to select ARCH_VEXPRESS unconditionally and come up with a better
> > > way to avoid having an empty platform list to make 'allnoconfig'
> > > still work.
> > 
> > The virtual guest platform support that Will and Marc did is small 
> > enough that it could always be selected in place of vexpress.
> 
> But that only helps when ARMv7 is selected, unless we want to build
> it only for ARMv4, v5 or v6 kernels.

That is still a problem.  I assumed from your statement above that only 
ARCH_MULTI_V7 was affected.

> Besides, the only reason we can't have a kernel without any platform
> selected is that the linker script has code in it to intentionally
> barf on that because it's guaranteed not to boot on any hardware.

I know -- that linker script assertion is mine.  :-)

> If we decide that building an allnoconfig without any platform
> is actually ok, we could just as well rip out that error statement.

Is it expected for allnoconfig kernels to still boot on other 
architectures? If so I think we should try to still provide a valid 
config even in the allnoconfig case.


Nicolas
Nicolas Pitre Jan. 9, 2013, 9:09 p.m. UTC | #9
On Wed, 9 Jan 2013, Rob Herring wrote:

> That patch is already posted, but Russell doesn't like it as you can
> have a kernel that doesn't boot. You don't like the allno and randconfig
> failures, so we're stuck. I think there are dozens of config options
> that will make you not boot on any given platform, so failing to boot
> because you did not select your machine is a non-issue.

Rather than simply accept that "there are dozens of config options that 
will make you not boot on any given platform" and allow for this to be 
used as an argument for future sloppiness, I think we should consider 
fixing the config rules to avoid that situation as much as possible 
instead.


Nicolas
Arnd Bergmann Jan. 9, 2013, 9:15 p.m. UTC | #10
On Wednesday 09 January 2013, Nicolas Pitre wrote:
> > If we decide that building an allnoconfig without any platform
> > is actually ok, we could just as well rip out that error statement.
> 
> Is it expected for allnoconfig kernels to still boot on other 
> architectures? If so I think we should try to still provide a valid 
> config even in the allnoconfig case.

The practical use of a kernel without any device drivers or file
systems is pretty low, I would not expect such a kernel to be
any useful. Having a regular configuration where you just disable
the platform support is a little different. This could happen
when the platform you are building for depends on a specific
feature to be enabled or disabled and that option is accidentally
changed. E.g. when you turn off MMU support on a platform that
depends on having that, we fall back to the default platform
(versatile?). The same may happen when you build for a single
board and one of it's dependencies goes away.

	Arnd
Nicolas Pitre Jan. 9, 2013, 9:37 p.m. UTC | #11
On Wed, 9 Jan 2013, Arnd Bergmann wrote:

> On Wednesday 09 January 2013, Nicolas Pitre wrote:
> > > If we decide that building an allnoconfig without any platform
> > > is actually ok, we could just as well rip out that error statement.
> > 
> > Is it expected for allnoconfig kernels to still boot on other 
> > architectures? If so I think we should try to still provide a valid 
> > config even in the allnoconfig case.
> 
> The practical use of a kernel without any device drivers or file
> systems is pretty low, I would not expect such a kernel to be
> any useful.

I didn't say "useful".  But at least "booting" e.g. to print out a 
kernel oops because no root filesystem is available or the like should 
be a minimum.  Having the wrong platform configured in might prevent 
even that from working, but at least we shouldn't allow for a kernel 
config that we know in advance has no chance of ever executing anywhere.


Nicolas
Arnd Bergmann Jan. 9, 2013, 10:14 p.m. UTC | #12
On Wednesday 09 January 2013, Nicolas Pitre wrote:
> On Wed, 9 Jan 2013, Arnd Bergmann wrote:
> 
> > On Wednesday 09 January 2013, Nicolas Pitre wrote:
> > > > If we decide that building an allnoconfig without any platform
> > > > is actually ok, we could just as well rip out that error statement.
> > > 
> > > Is it expected for allnoconfig kernels to still boot on other 
> > > architectures? If so I think we should try to still provide a valid 
> > > config even in the allnoconfig case.
> > 
> > The practical use of a kernel without any device drivers or file
> > systems is pretty low, I would not expect such a kernel to be
> > any useful.
> 
> I didn't say "useful".  But at least "booting" e.g. to print out a 
> kernel oops because no root filesystem is available or the like should 
> be a minimum.  Having the wrong platform configured in might prevent 
> even that from working, but at least we shouldn't allow for a kernel 
> config that we know in advance has no chance of ever executing anywhere.

x86 allows you disable support for all the CPU types at the same time,
although the default is to not give you the option and enable them
all together, which means that allnoconfig may still boot, but
randconfig may not. That is probably a reasonable compromise.

Maybe we could have the virtual platform as the default but make it
invisible if CONFIG_EXPERT is disable. That would also make allnoconfig
be reasonable. Thinking a bit more about the CPU setting for the
virtual platform, I guess it actually makes sense to let the user
build this for any CPU: It makes a very nice minimal kernel for
running in qemu and easily allows you to test a rootfs that was
built for an arbitrary CPU without having to support a specific
SoC in qemu.

	Arnd
Rob Herring Jan. 10, 2013, 3:51 a.m. UTC | #13
On 01/09/2013 02:48 PM, Russell King - ARM Linux wrote:
> On Wed, Jan 09, 2013 at 02:39:36PM -0600, Rob Herring wrote:
>> On 01/09/2013 02:22 PM, Arnd Bergmann wrote:
>>> On Wednesday 09 January 2013, Nicolas Pitre wrote:
>>>> On Wed, 9 Jan 2013, Arnd Bergmann wrote:
>>>>
>>>>> On a related topic, I still think we should fix ARCH_MULTI_V7 not
>>>>> to select ARCH_VEXPRESS unconditionally and come up with a better
>>>>> way to avoid having an empty platform list to make 'allnoconfig'
>>>>> still work.
>>>>
>>>> The virtual guest platform support that Will and Marc did is small 
>>>> enough that it could always be selected in place of vexpress.
>>>
>>> But that only helps when ARMv7 is selected, unless we want to build
>>> it only for ARMv4, v5 or v6 kernels.
>>>
>>> Besides, the only reason we can't have a kernel without any platform
>>> selected is that the linker script has code in it to intentionally
>>> barf on that because it's guaranteed not to boot on any hardware.
>>>
>>> If we decide that building an allnoconfig without any platform
>>> is actually ok, we could just as well rip out that error statement.
>>>
>>
>> That patch is already posted, but Russell doesn't like it as you can
>> have a kernel that doesn't boot. You don't like the allno and randconfig
>> failures, so we're stuck. I think there are dozens of config options
>> that will make you not boot on any given platform, so failing to boot
>> because you did not select your machine is a non-issue.
> 
> What I actually suggested is that we should be aiming for the DT side
> of things to get to the point where DT is just another _single_ platform
> as far as that code goes, and that DT should describe the hardware
> sufficiently well that we don't have multiple machine_desc things to
> select via DT - so a DT kernel would have exactly one machine_desc (or
> maybe even zero! - with the linker script check conditional on !CONFIG_OF)
> 
> That then gets rid of the issue entirely.

I agree completely. I look at mach-highbank (and mach-virt) as what else
needs to be done in terms of refactoring or moving to DT. That's
certainly a long term goal, but what is the short term solution?

Rob
Arnd Bergmann Jan. 10, 2013, 10:16 a.m. UTC | #14
On Thursday 10 January 2013, Rob Herring wrote:
> > 
> > What I actually suggested is that we should be aiming for the DT side
> > of things to get to the point where DT is just another single platform
> > as far as that code goes, and that DT should describe the hardware
> > sufficiently well that we don't have multiple machine_desc things to
> > select via DT - so a DT kernel would have exactly one machine_desc (or
> > maybe even zero! - with the linker script check conditional on !CONFIG_OF)
> > 
> > That then gets rid of the issue entirely.
> 
> I agree completely. I look at mach-highbank (and mach-virt) as what else
> needs to be done in terms of refactoring or moving to DT.

It's also what we are doing on arm64 from the start, where there is
no machine descriptor already. Any experience we make with arm64 will
also help us improve arm in the long run, I hope.

> That's certainly a long term goal, but what is the short term solution?

I'd certainly be happy with

* removing the intentional build error for ARCH_MULTIPLATFORM, but leaving
  it in for !ARCH_MULTIPLATFORM
* having mach-virt enabled by default on ARCH_MULTIPLATFORM, and only
  visible for EXPERT.
* making mach-virt compatible with all CPUs starting with ARM7.

That would make allnoconfig, allyesconfig and allmodconfig work (besides
all the other bugs) on qemu at least, avoid build errors with randconfig
and make it hard enough to build a kernel that doesn't run on anything.

	Arnd
Christopher Covington Jan. 10, 2013, 1:20 p.m. UTC | #15
On 01/10/2013 05:16 AM, Arnd Bergmann wrote:

[...]

> I'd certainly be happy with
> 
> * removing the intentional build error for ARCH_MULTIPLATFORM, but leaving
>   it in for !ARCH_MULTIPLATFORM
> * having mach-virt enabled by default on ARCH_MULTIPLATFORM, and only
>   visible for EXPERT.
> * making mach-virt compatible with all CPUs starting with ARM7.
> 
> That would make allnoconfig, allyesconfig and allmodconfig work (besides
> all the other bugs) on qemu at least, avoid build errors with randconfig
> and make it hard enough to build a kernel that doesn't run on anything.

If this is going to be the approach, I would like to once again suggest that
paths, configuration options, and compatibility strings along the lines of
"mach-genericarmv7", "Generic ARMv7 Machine", and "linux,genericarmv7" be
considered in place of "mach-virt", "Dummy Virtual Machine" and
"linux,dummy-virt", respectively.

Virtualization may have been the initial motivation for creating mach-virt,
but the code is clearly also immediately applicable to other environments such
as simulation/emulation, and sticking to a name like "Dummy Virtual Machine"
makes it sound like such usage is erroneous.

Regards,
Christopher
Marc Zyngier Jan. 10, 2013, 1:45 p.m. UTC | #16
On 10/01/13 13:20, Christopher Covington wrote:
> On 01/10/2013 05:16 AM, Arnd Bergmann wrote:
> 
> [...]
> 
>> I'd certainly be happy with
>>
>> * removing the intentional build error for ARCH_MULTIPLATFORM, but leaving
>>   it in for !ARCH_MULTIPLATFORM
>> * having mach-virt enabled by default on ARCH_MULTIPLATFORM, and only
>>   visible for EXPERT.
>> * making mach-virt compatible with all CPUs starting with ARM7.
>>
>> That would make allnoconfig, allyesconfig and allmodconfig work (besides
>> all the other bugs) on qemu at least, avoid build errors with randconfig
>> and make it hard enough to build a kernel that doesn't run on anything.
> 
> If this is going to be the approach, I would like to once again suggest that
> paths, configuration options, and compatibility strings along the lines of
> "mach-genericarmv7", "Generic ARMv7 Machine", and "linux,genericarmv7" be
> considered in place of "mach-virt", "Dummy Virtual Machine" and
> "linux,dummy-virt", respectively.
> 
> Virtualization may have been the initial motivation for creating mach-virt,
> but the code is clearly also immediately applicable to other environments such
> as simulation/emulation, and sticking to a name like "Dummy Virtual Machine"
> makes it sound like such usage is erroneous.

mach-virt requires GIC, Architected Timers and PSCI. I'm not prepared to
impose these requirements on all future v7 platforms yet.

When we reach a point where we have a completely DT driven setup,
without any explicit hook, I'll be happy to add new compatibility strings.

	M.
diff mbox

Patch

diff --git a/arch/arm/Kconfig b/arch/arm/Kconfig
index 335e220..3357d0b 100644
--- a/arch/arm/Kconfig
+++ b/arch/arm/Kconfig
@@ -322,6 +322,11 @@  config ARCH_VERSATILE
 	help
 	  This enables support for ARM Ltd Versatile board.
 
+config ARCH_VEXPRESS_NOMMU
+	bool "ARM Ltd. Versatile Express family for NOMMU"
+	depends on !MMU
+	select ARCH_VEXPRESS
+
 config ARCH_AT91
 	bool "Atmel AT91"
 	select ARCH_REQUIRE_GPIOLIB