>From 365e3a59c34055490f137af2da281d4305535237 Mon Sep 17 00:00:00 2001
From: Jacob Pan <[email protected]>
Date: Thu, 11 Jun 2009 09:42:42 -0700
Subject: [PATCH] x86/moorestown: add moorestown platform flags
This patch adds some new features to the x86 platform feature flags.
These new features are first introduced in Intel Moorestown platform but
may continue to be present in the next generations.
Feature flags are initialized based on HW subarchitecture ID provided
by the FW via boot protocol.
Signed-off-by: Jacob Pan <[email protected]>
---
arch/x86/Kconfig | 13 ++++++++++++-
arch/x86/include/asm/platform_feature.h | 2 ++
arch/x86/kernel/platform_info.c | 12 +++++++++++-
3 files changed, 25 insertions(+), 2 deletions(-)
diff --git a/arch/x86/Kconfig b/arch/x86/Kconfig
index d1430ef..c48c291 100644
--- a/arch/x86/Kconfig
+++ b/arch/x86/Kconfig
@@ -1658,7 +1658,18 @@ config CMDLINE_OVERRIDE
This is used to work around broken boot loaders. This should
be set to 'N' under normal conditions.
-
+config MRST
+ bool "Moorestown MID platform"
+ default n
+ depends on X86_32
+ select SFI
+ help
+ Moorestown is Intel's Low Power Intel Architecture (LPIA) based Moblin
+ Internet Device(MID) platform. Moorestown consists of two chips:
+ Lincroft (CPU core, graphics, and memory controller) and Langwell IOH.
+ Unlike standard x86 PCs, Moorestown does not have many legacy devices
+ nor standard legacy replacement devices/features. e.g. Moorestown does
+ not contain i8259, i8254, HPET, legacy BIOS, most of the io ports.
endmenu
config ARCH_ENABLE_MEMORY_HOTPLUG
diff --git a/arch/x86/include/asm/platform_feature.h b/arch/x86/include/asm/platform_feature.h
index bcadda5..026bdc7 100644
--- a/arch/x86/include/asm/platform_feature.h
+++ b/arch/x86/include/asm/platform_feature.h
@@ -45,6 +45,8 @@
#define X86_PLATFORM_FEATURE_ACPI (0*32+8) /* has ACPI support */
#define X86_PLATFORM_FEATURE_SFI (0*32+9) /* has SFI support */
#define X86_PLATFORM_FEATURE_8042 (0*32+10) /* i8042 KBC */
+#define X86_PLATFORM_FEATURE_APBT (0*32+11) /* APB timer */
+#define X86_PLATFORM_FEATURE_VRTC (0*32+12) /* virtual RTC */
extern __u32 platform_feature[N_PLATFORM_CAPINTS];
extern const char *const
diff --git a/arch/x86/kernel/platform_info.c b/arch/x86/kernel/platform_info.c
index b39898a..3021ea9 100644
--- a/arch/x86/kernel/platform_info.c
+++ b/arch/x86/kernel/platform_info.c
@@ -105,7 +105,17 @@ static int __init sysfs_platforminfo_init(void)
}
arch_initcall(sysfs_platforminfo_init);
+/* Initialize a set of default feature flags based on subarch IDs
+ * Currently, only MRST platform has non-X86 PC standard feature set.
+ */
void platform_feature_init_default(int subarch_id)
{
- printk(KERN_INFO "Use default X86 platform feature set\n");
+ if ((subarch_id >= 0) && (subarch_id < N_X86_SUBARCHS)) {
+ if (subarch_id == X86_SUBARCH_MRST) {
+ setup_mrst_default_feature();
+ return;
+ }
+ } else {
+ printk(KERN_INFO "Use default X86 platform feature set\n");
+ }
}
--
1.5.6.5
* Pan, Jacob jun <[email protected]> wrote:
> >From 365e3a59c34055490f137af2da281d4305535237 Mon Sep 17 00:00:00 2001
> From: Jacob Pan <[email protected]>
> Date: Thu, 11 Jun 2009 09:42:42 -0700
> Subject: [PATCH] x86/moorestown: add moorestown platform flags
>
> This patch adds some new features to the x86 platform feature flags.
>
> These new features are first introduced in Intel Moorestown platform but
> may continue to be present in the next generations.
>
> Feature flags are initialized based on HW subarchitecture ID provided
> by the FW via boot protocol.
>
> Signed-off-by: Jacob Pan <[email protected]>
> ---
> arch/x86/Kconfig | 13 ++++++++++++-
> arch/x86/include/asm/platform_feature.h | 2 ++
> arch/x86/kernel/platform_info.c | 12 +++++++++++-
> 3 files changed, 25 insertions(+), 2 deletions(-)
>
> diff --git a/arch/x86/Kconfig b/arch/x86/Kconfig
> index d1430ef..c48c291 100644
> --- a/arch/x86/Kconfig
> +++ b/arch/x86/Kconfig
> @@ -1658,7 +1658,18 @@ config CMDLINE_OVERRIDE
>
> This is used to work around broken boot loaders. This should
> be set to 'N' under normal conditions.
> -
> +config MRST
> + bool "Moorestown MID platform"
> + default n
> + depends on X86_32
> + select SFI
> + help
> + Moorestown is Intel's Low Power Intel Architecture (LPIA) based Moblin
> + Internet Device(MID) platform. Moorestown consists of two chips:
> + Lincroft (CPU core, graphics, and memory controller) and Langwell IOH.
> + Unlike standard x86 PCs, Moorestown does not have many legacy devices
> + nor standard legacy replacement devices/features. e.g. Moorestown does
> + not contain i8259, i8254, HPET, legacy BIOS, most of the io ports.
> endmenu
>
> config ARCH_ENABLE_MEMORY_HOTPLUG
> diff --git a/arch/x86/include/asm/platform_feature.h b/arch/x86/include/asm/platform_feature.h
> index bcadda5..026bdc7 100644
> --- a/arch/x86/include/asm/platform_feature.h
> +++ b/arch/x86/include/asm/platform_feature.h
> @@ -45,6 +45,8 @@
> #define X86_PLATFORM_FEATURE_ACPI (0*32+8) /* has ACPI support */
> #define X86_PLATFORM_FEATURE_SFI (0*32+9) /* has SFI support */
> #define X86_PLATFORM_FEATURE_8042 (0*32+10) /* i8042 KBC */
> +#define X86_PLATFORM_FEATURE_APBT (0*32+11) /* APB timer */
> +#define X86_PLATFORM_FEATURE_VRTC (0*32+12) /* virtual RTC */
>
> extern __u32 platform_feature[N_PLATFORM_CAPINTS];
> extern const char *const
> diff --git a/arch/x86/kernel/platform_info.c b/arch/x86/kernel/platform_info.c
> index b39898a..3021ea9 100644
> --- a/arch/x86/kernel/platform_info.c
> +++ b/arch/x86/kernel/platform_info.c
> @@ -105,7 +105,17 @@ static int __init sysfs_platforminfo_init(void)
> }
> arch_initcall(sysfs_platforminfo_init);
>
> +/* Initialize a set of default feature flags based on subarch IDs
> + * Currently, only MRST platform has non-X86 PC standard feature set.
> + */
please use the customary comment style:
/*
* Comment .....
* ...... goes here:
*/
specified in Documentation/CodingStyle.
> void platform_feature_init_default(int subarch_id)
> {
> - printk(KERN_INFO "Use default X86 platform feature set\n");
> + if ((subarch_id >= 0) && (subarch_id < N_X86_SUBARCHS)) {
> + if (subarch_id == X86_SUBARCH_MRST) {
> + setup_mrst_default_feature();
> + return;
> + }
> + } else {
> + printk(KERN_INFO "Use default X86 platform feature set\n");
> + }
> }
Why dont we have some clean and robust PCI config space based
enumeration instead of this boot ID based thing?
Ingo
> Why dont we have some clean and robust PCI config space based
> enumeration instead of this boot ID based thing?
That strikes me as a rather dumb question given that
- Embedded x86 like devices are going to regularly occur without any PCI
- You need to know the platform in order to know how to access any PCI
bus that may or may not hypothetically exist.
Take a look at how ARM and PPC address this. They do it via platform
features and descriptors for good reason.
One thing PPC does nice is to encapsulate pretty much all of it into a
struct of function pointers for each platform to keep the if conditionals
down. The ARM file layout of arch/foo/platform/bar might also be a good
idea before mrst and olpc and other stuff like this starts to make the
main x86 directory hard to read ?
Alan
* Alan Cox <[email protected]> wrote:
> > Why dont we have some clean and robust PCI config space based
> > enumeration instead of this boot ID based thing?
>
> That strikes me as a rather dumb question given that
It's an entirely legitimate question, given that many other current
modern subarchitectures detect themselves based on PCI config space
early accesses or (sometimes) BIOS data structures - and that both
of those methods are better than using boot flags.
> - Embedded x86 like devices are going to regularly occur without
> any PCI
This proposed Intel subarchitecture comes with PCI support,
obviously.
> - You need to know the platform in order to know how to access any
> PCI bus that may or may not hypothetically exist.
Uhm, not really.
Have a look at arch/x86/kernel/early-quirks.c. All you generally
need to know is a PCI ID that sits on the root bus.
Early PCI ID checks are typical and robust way to identify 'weird'
subarchitectures. Sometimes we do it via BIOS data structures. Only
as a last option do we want to use boot loader mechanisms - it's the
most inflexible one really.
Furtherore, Moorestown comes with SFI and there sure can be a BIOS
table that describes the platform properly. We can read such tables
very early during bootup, well before platform devices are set up.
Ingo
> It's an entirely legitimate question, given that many other current
> modern subarchitectures detect themselves based on PCI config space
> early accesses or (sometimes) BIOS data structures - and that both
> of those methods are better than using boot flags.
The flags are passed by the boot loader which is the one thing that knows
what the platform is only deeply embedded hardware. See the ARM and PPC
ports.
> > - Embedded x86 like devices are going to regularly occur without
> > any PCI
>
> This proposed Intel subarchitecture comes with PCI support,
> obviously.
And this is relevant to the fact there will be systems without PCI how ?
> > - You need to know the platform in order to know how to access any
> > PCI bus that may or may not hypothetically exist.
>
> Uhm, not really.
Uhm yes really
> Have a look at arch/x86/kernel/early-quirks.c. All you generally
> need to know is a PCI ID that sits on the root bus.
How will you probe the root bus without knowing what the PCI
configuration mechanism is for your platform
> Early PCI ID checks are typical and robust way to identify 'weird'
> subarchitectures. Sometimes we do it via BIOS data structures. Only
> as a last option do we want to use boot loader mechanisms - it's the
> most inflexible one really.
SFI doesn't indicate MRST
>
> Furtherore, Moorestown comes with SFI and there sure can be a BIOS
> table that describes the platform properly. We can read such tables
> very early during bootup, well before platform devices are set up.
But they don't tell you what the platform is. Again see every platform we
currently support which has deeply embedded forms. There is a reason they
went that way and it includes fifteen years of finding out which ways
don't work for all cases. For example if you don't have standard PC like
firmware and you go looking for SFI or ACPI will your box stay up ?
It's also kind of irrelevant to the flags anyway - you need the boot
flags whether the platform comes from the bootloader or divine
intervention somewhere early in probing.
There is a more general question which is whether the flags are better
than the PPC style approach of having a machine vector and putting them
in that so you just do
platform_ops = platform_ops_table[our_platform]
and use
platform_ops->
for the various methods such as EBDA and which timer methods to use
* Alan Cox <[email protected]> wrote:
> > It's an entirely legitimate question, given that many other
> > current modern subarchitectures detect themselves based on PCI
> > config space early accesses or (sometimes) BIOS data structures
> > - and that both of those methods are better than using boot
> > flags.
>
> The flags are passed by the boot loader which is the one thing
> that knows what the platform is only deeply embedded hardware. See
> the ARM and PPC ports.
And? There's an obvious quality difference between various platform
enumeration methods - and we strive for the highest quality methods.
Using boot flags is one of the lowest quality enumeration methods
and the fact that there's precedence for it in other architectures
is not a technical reason to make the same mistakes on x86 too.
Especially here where there's two other enumeration methods easily
available: SFI and PCI. Any of those suffices.
> > > - Embedded x86 like devices are going to regularly occur
> > > without any PCI
> >
> > This proposed Intel subarchitecture comes with PCI support,
> > obviously.
>
> And this is relevant to the fact there will be systems without PCI
> how ?
It is obviously relevant to the fact that the proposed patches here
(on which i commented) come for a platform with PCI support. Or do
you claim that Intel submitted this patch-set for PCI-less MIDs?
(this was not mentioned anywhere in the patches)
> > > - You need to know the platform in order to know how to access
> > > any PCI bus that may or may not hypothetically exist.
> >
> > Uhm, not really.
>
> Uhm yes really
>
> > Have a look at arch/x86/kernel/early-quirks.c. All you generally
> > need to know is a PCI ID that sits on the root bus.
>
> How will you probe the root bus without knowing what the PCI
> configuration mechanism is for your platform
That's very simple really - there's basically a very low number of
PCI configuration mechanisms/ports on x86, and the MIDs have no
reason to depart from that standard. There's two that matter in
practice, and it can all be safely probed.
PCI is really essential to any modern architecture (precisely
because it's standard and well-known) and i doubt Intel will try to
engineer out PCI from its systems. If it happens i will comment on
that kind of braindamage in due course.
But, PCI IDs dont _have_ to be used - there's ample other
environmental space available via SFI, ACPI or EFI or old-and-proven
EBDA.
> > Early PCI ID checks are typical and robust way to identify
> > 'weird' subarchitectures. Sometimes we do it via BIOS data
> > structures. Only as a last option do we want to use boot loader
> > mechanisms - it's the most inflexible one really.
>
> SFI doesn't indicate MRST
So do you claim that this particular patchset supports systems with
no SFI, no ACPI, just plain bootloader flags?
> > Furtherore, Moorestown comes with SFI and there sure can be a
> > BIOS table that describes the platform properly. We can read
> > such tables very early during bootup, well before platform
> > devices are set up.
>
> But they don't tell you what the platform is. Again see every
> platform we currently support which has deeply embedded forms.
> There is a reason they went that way and it includes fifteen years
> of finding out which ways don't work for all cases. For example if
> you don't have standard PC like firmware and you go looking for
> SFI or ACPI will your box stay up ?
Do you claim that the Intel patchset here deals with systems that
have no SFI and no ACPI? My question was very specific to this
patch-set. Bootloader flags suck because they add a needless
dimension to the already complex boot protocol. Use existing
mechanisms instead - it's really easy and has other advantages as
well.
Non-x86 ultra-embedded might use crappier techniques but i'd expect
Intel has the resources to do better here. Using standard hardware
or firmware interfaces for that is far better in the x86 space and
we have no reason to depart from that really.
Ingo
> > The flags are passed by the boot loader which is the one thing
> > that knows what the platform is only deeply embedded hardware. See
> > the ARM and PPC ports.
>
> And? There's an obvious quality difference between various platform
> enumeration methods - and we strive for the highest quality methods.
Good the boot loader knows precisely what it is running on
> Using boot flags is one of the lowest quality enumeration methods
It's probably the most reliable. If you don't believe so then provide
data to back your assertion
> and the fact that there's precedence for it in other architectures
> is not a technical reason to make the same mistakes on x86 too.
How about "they tried other methods and they didn't work"
> It is obviously relevant to the fact that the proposed patches here
> (on which i commented) come for a platform with PCI support. Or do
> you claim that Intel submitted this patch-set for PCI-less MIDs?
> (this was not mentioned anywhere in the patches)
I claimed nothing of the sort Ingo. I pointed out your approach doesn't
actually work except for the PCI case. WHich means it doesn't even work
on a generic 486 ISA bus PC. Is that a single chip embedded device using
a licenced core from one of the 486 vendors or an ISA PC .. how do you
tell.
> That's very simple really - there's basically a very low number of
> PCI configuration mechanisms/ports on x86, and the MIDs have no
> reason to depart from that standard. There's two that matter in
> practice, and it can all be safely probed.
You try one, that I/O address isn't mapped and you get a fatal machine
check. Exactly why is that reliable ?
> PCI is really essential to any modern architecture (precisely
> because it's standard and well-known) and i doubt Intel will try to
> engineer out PCI from its systems. If it happens i will comment on
> that kind of braindamage in due course.
PCI is already obsolete.
> But, PCI IDs dont _have_ to be used - there's ample other
> environmental space available via SFI, ACPI or EFI or old-and-proven
> EBDA.
You only have an EBDA on a PC type machine. You only have ACPI on a PC
type machien. You only have EFI ona PCI type machine. So how will you
look for them when the generic whacky BIOS windows for 16bit DOS
compatibility that hold these things might be ROM or random chunks of
main memory ?
> > SFI doesn't indicate MRST
>
> So do you claim that this particular patchset supports systems with
> no SFI, no ACPI, just plain bootloader flags?
If you have the bootloader pass the information then that is one source
of it. It doesn't need to be the only one.
You are also pointlessly entangling MRST and platform identification.
Please separate out the following in your thinking
- How you implement int platform = gee_what_the_hell_am_i_running_on()
and "ok so I'm a voyager, now what"
> Do you claim that the Intel patchset here deals with systems that
> have no SFI and no ACPI? My question was very specific to this
> patch-set.
I'm trying to sort out the broader question.
It's called architecture - planning for the future, design. If you want
to be utterly narrow minded you can NIH about every platform but x86 and
pretend hardware won't exist that doesn't fit your predetermined vision.
If you don't plan for the future you get to write the arch code every new
hardware platform. Which is fun and you really want to spend your life
re-inventing this every six months no ?
> Non-x86 ultra-embedded might use crappier techniques but i'd expect
> Intel has the resources to do better here. Using standard hardware
What makes you think x86 ultra-embedded is solely an Intel thing. OLPC
isn't Intel for one although its still quite PCish.
> or firmware interfaces for that is far better in the x86 space and
> we have no reason to depart from that really.
It isn't about departing from that, it's about supporting the future. If
you untangle this into two questions
- How do I identify this platform
- How do I run on lots of platforms without the code becoming a
mess
it might make more progress.
The important one right now is the second question. Whether you set the
platform on dip switches or by PCI bus probing the interface doesn't
change. Its a function that returns a number.
The hardware interfaces don't exist in x86 unlike most other
architectures so that does need addressing properly and can't be deferred.
* Alan Cox <[email protected]> wrote:
> > > The flags are passed by the boot loader which is the one thing
> > > that knows what the platform is only deeply embedded hardware. See
> > > the ARM and PPC ports.
> >
> > And? There's an obvious quality difference between various
> > platform enumeration methods - and we strive for the highest
> > quality methods.
>
> Good the boot loader knows precisely what it is running on
That's a pretty bogus claim - on x86 a bootloader generally knows
very little about 'what it is running on'. We do most of the
enumeration in early platform code and retrieve information via
standard BIOS interfaces.
> > Using boot flags is one of the lowest quality enumeration
> > methods
>
> It's probably the most reliable. If you don't believe so then
> provide data to back your assertion
You are the one who is trying to do a change here really, so you
should provide data to back your assertion that all existing x86
enumeration methods are wrong and that modern x86 platforms from
now on should freely splinter into non-standard platforms along
boot flags.
The success of the PC platform was based on standardization and
standard interfaces. Do you need data for that fact? ;-)
> > and the fact that there's precedence for it in other
> > architectures is not a technical reason to make the same
> > mistakes on x86 too.
>
> How about "they tried other methods and they didn't work"
The thing is, you are trying to defend a v1 patch-set here that is
really indefensible: it's ugly and deficient in numerous smaller and
larger details. I outlined numerous deficiencies already - and i'll
review v2 too to see what else is there to fix.
You outlined here various claims about how x86 should suddenly
change into ARM or PPC to become 'successful' - but if this current
patch-set is your attempt at that then it has failed spectacularly.
Thanks,
Ingo
> That's a pretty bogus claim - on x86 a bootloader generally knows
> very little about 'what it is running on'. We do most of the
> enumeration in early platform code and retrieve information via
> standard BIOS interfaces.
Stop thinking about existing x86 PC systems running grub for a bit.
Highly embedded systems usually boot from ROM firmware direct into the
kernel. They have a very good idea what they are running on because they
were flashed specifically for the device.
I'm not worried about PC/ISA v PC/MCA v Voyager v CBus v CompaqSMP v
Intel MP 1.4 SMP v PCI v PCI/X v PCI Express v OLPC v Voyager etc, we can
clearly tell all those apart via bus probing. But those are all basically
a PC.
> > How about "they tried other methods and they didn't work"
>
> The thing is, you are trying to defend a v1 patch-set here that is
> really indefensible: it's ugly and deficient in numerous smaller and
> larger details. I outlined numerous deficiencies already - and i'll
> review v2 too to see what else is there to fix.
No I'm trying to understand what you actually want the thing to look like.
Are we talking
/* Fixed struct not pointer for speed */
struct platform_ops platform_ops;
memcpy(platform_ops, platform_op_list[detect_platform_type()],
sizeof(struct platform_ops));
platform->add_private_resources();
platform->timer_foo();
platfomr->timer_bar();
or
platform->init();
timer = platform->timer;
timer->begin()
* Alan Cox <[email protected]> wrote:
> > That's a pretty bogus claim - on x86 a bootloader generally
> > knows very little about 'what it is running on'. We do most of
> > the enumeration in early platform code and retrieve information
> > via standard BIOS interfaces.
>
> Stop thinking about existing x86 PC systems running grub for a
> bit. [...]
You are talking to the wrong person then i guess, i'm not going to
ignore 95% of our installed base. ;-)
We will gladly take clean x86 patches that abstract away lowlevel
details of x86 platforms, and have been taking them and have been
writing them for a long time. If this patch-set can shape itself in
such a way (as i requested), without hindering the common case, it
is certainly welcome.
Generally, if you try to deviate from de facto standards then you
better show a very good reason and much cleaner code than this
deficient v1 patch-set.
Yes, platform abstraction is good if it's necessary and if it's done
cleanly, and x86 certainly has a few things to learn in that area
(it still has way too much platform spaghetti), but i'm not
convinced here at all that it's necessary, and it's certainly not
cleanly done at all. As i pointed it out in my review in detail,
it's full of 'if (feature)' crap invading core platform code for
example.
There's good examples as well, from the same quarters of Intel: for
example i reviewed and commented on the related SFI patches a few
days ago, and they looked distinctly clean and desirable.
Thanks,
Ingo
> We will gladly take clean x86 patches that abstract away lowlevel
> details of x86 platforms, and have been taking them and have been
> writing them for a long time. If this patch-set can shape itself in
> such a way (as i requested), without hindering the common case, it
> is certainly welcome.
Lets try again shall we. I'l repeat the relevant bits of the mail you
quoted bits from and ignored almost all of
> The thing is, you are trying to defend a v1 patch-set here that is
> really indefensible: it's ugly and deficient in numerous smaller and
> larger details. I outlined numerous deficiencies already - and i'll
> review v2 too to see what else is there to fix.
No I'm trying to understand what you actually want the thing to look like.
Are we talking
/* Fixed struct not pointer for speed */
struct platform_ops platform_ops;
memcpy(platform_ops, platform_op_list[detect_platform_type()],
sizeof(struct platform_ops));
platform->add_private_resources();
platform->timer_foo();
platfomr->timer_bar();
or
platform->init();
timer = platform->timer;
timer->begin()
* Alan Cox <[email protected]> wrote:
> > The thing is, you are trying to defend a v1 patch-set here that
> > is really indefensible: it's ugly and deficient in numerous
> > smaller and larger details. I outlined numerous deficiencies
> > already - and i'll review v2 too to see what else is there to
> > fix.
>
> No I'm trying to understand what you actually want the thing to
> look like.
It's a case by case thing and i pointed out a few specific
directions in the review. The IO-APIC changes should probably go on
top of Jeremy's IO-APIC driver-ization patches. They dont
necessarily need their own IO-APIC driver (if the resulting line
count increase is too much), but they should not wreck Jeremy's
IO-APIC patches.
Bootup quirks that are small modificatons to existing PC
initialization sequences should go into x86_quirks.
Timer related changes (the APB system timer) are mostly modular
already by virtue of half of it being a clocksource and clockevents
driver. The remaining bit of system timer handling should be
abstracted out as a 'struct x86_system_timer' kind of structure,
with ->init, ->timer_irq and ->shutdown functions.
[ Although it is beyond me why ABP was done - why wasnt HPET good
enough? HPET can do per CPU clockevents too and it's just as
off-chip (and hence fundamentally slow) as ABP. ]
Ingo
* Alan Cox <[email protected]> wrote:
> > We will gladly take clean x86 patches that abstract away
> > lowlevel details of x86 platforms, and have been taking them and
> > have been writing them for a long time. If this patch-set can
> > shape itself in such a way (as i requested), without hindering
> > the common case, it is certainly welcome.
>
> Lets try again shall we. I'l repeat the relevant bits of the mail
> you quoted bits from and ignored almost all of
Below is my reply to your above points (already sent in another
mail) - which is really just mostly re-hashing the (rather obvious
looking) things that have been said before.
Ingo
---------------------->
* Alan Cox <[email protected]> wrote:
> > The thing is, you are trying to defend a v1 patch-set here that
> > is really indefensible: it's ugly and deficient in numerous
> > smaller and larger details. I outlined numerous deficiencies
> > already - and i'll review v2 too to see what else is there to
> > fix.
>
> No I'm trying to understand what you actually want the thing to
> look like.
It's a case by case thing and i pointed out a few specific
directions in the review. The IO-APIC changes should probably go on
top of Jeremy's IO-APIC driver-ization patches. They dont
necessarily need their own IO-APIC driver (if the resulting line
count increase is too much), but they should not wreck Jeremy's
IO-APIC patches.
Bootup quirks that are small modificatons to existing PC
initialization sequences should go into x86_quirks.
Timer related changes (the APB system timer) are mostly modular
already by virtue of half of it being a clocksource and clockevents
driver. The remaining bit of system timer handling should be
abstracted out as a 'struct x86_system_timer' kind of structure,
with ->init, ->timer_irq and ->shutdown functions.
[ Although it is beyond me why ABP was done - why wasnt HPET good
enough? HPET can do per CPU clockevents too and it's just as
off-chip (and hence fundamentally slow) as ABP. ]
Ingo
* Ingo Molnar <[email protected]> wrote:
> And? There's an obvious quality difference between various
> platform enumeration methods - and we strive for the highest
> quality methods.
>
> Using boot flags is one of the lowest quality enumeration methods
> and the fact that there's precedence for it in other architectures
> is not a technical reason to make the same mistakes on x86 too.
>
> Especially here where there's two other enumeration methods easily
> available: SFI and PCI. Any of those suffices.
btw., the "I am MRST" bootstrap info is certainly doable via a
bootloader flag as well, if this platform is _so_ deprived of basic
PC features that it has no other channel of information. Especially
if it has no BIOS and if the bootloader provides the memory map as
well - which seems to be the case here.
I can understand the PCI ID space being potentially awkward (if many
models with mismatching PCI IDs are planned) - so it's certainly
possible that the only thing that remains in the end is the
bootloader provided flag - no matter how sucky that may be.
So i am not trying to make a bigger deal out of this than it really
deserves and this is not a showstopper in my eyes - but this is
really an exceptional case and shouldnt be the glory model for how
ultra embedded should be done on x86 ...
Ingo
On Fri, 26 Jun 2009, Ingo Molnar wrote:
> [ Although it is beyond me why ABP was done - why wasnt HPET good
> enough? HPET can do per CPU clockevents too and it's just as
> off-chip (and hence fundamentally slow) as ABP. ]
Welcome to the wonderful world of embedded systems. Just have a peek
into arch/[arm/powerpc/mips] to see what's coming up to us with full
force. I would not be surprised when we see an x86 system sharing the
device driver for i2c or whatever with an ARM SoC in the foreseable
future.
Thanks,
tglx
On Fri, 26 Jun 2009 09:19:55 +0200
Ingo Molnar <[email protected]> wrote:
> > void platform_feature_init_default(int subarch_id)
> > {
> > - printk(KERN_INFO "Use default X86 platform feature set\n");
> > + if ((subarch_id >= 0) && (subarch_id < N_X86_SUBARCHS)) {
> > + if (subarch_id == X86_SUBARCH_MRST) {
> > + setup_mrst_default_feature();
> > + return;
> > + }
> > + } else {
> > + printk(KERN_INFO "Use default X86 platform feature
> > set\n");
> > + }
> > }
>
> Why dont we have some clean and robust PCI config space based
> enumeration instead of this boot ID based thing?
MRST has special PCI support that depends on having the platform
identified already, so if you want to use something other than a flag
here, maybe we could use an SFI string instead.
--
Jesse Barnes, Intel Open Source Technology Center
On Fri, 26 Jun 2009 18:32:42 +0200 (CEST)
Thomas Gleixner <[email protected]> wrote:
> On Fri, 26 Jun 2009, Ingo Molnar wrote:
> > [ Although it is beyond me why ABP was done - why wasnt HPET good
> > enough? HPET can do per CPU clockevents too and it's just as
> > off-chip (and hence fundamentally slow) as ABP. ]
>
> Welcome to the wonderful world of embedded systems. Just have a peek
> into arch/[arm/powerpc/mips] to see what's coming up to us with full
> force. I would not be surprised when we see an x86 system sharing the
> device driver for i2c or whatever with an ARM SoC in the foreseable
> future.
Ha, yeah I was just going to say "think embedded". ABP is a much
simpler spec and programming interface than HPET, and since we were
designing new custom silicon, it made sense to just do the simple
thing, rather than butchering an existing spec, then making a partial
HPET that looks like ABP anyway and forcing any future HPET updates to
conform to the new standard (very similar reasoning to the ACPI vs SFI
discussion btw). Hopefully the technologies we've come up with for
MRST will be flexible enough to be used in future platforms as well;
we'll see.
--
Jesse Barnes, Intel Open Source Technology Center
* Jesse Barnes <[email protected]> wrote:
> On Fri, 26 Jun 2009 09:19:55 +0200
> Ingo Molnar <[email protected]> wrote:
> > > void platform_feature_init_default(int subarch_id)
> > > {
> > > - printk(KERN_INFO "Use default X86 platform feature set\n");
> > > + if ((subarch_id >= 0) && (subarch_id < N_X86_SUBARCHS)) {
> > > + if (subarch_id == X86_SUBARCH_MRST) {
> > > + setup_mrst_default_feature();
> > > + return;
> > > + }
> > > + } else {
> > > + printk(KERN_INFO "Use default X86 platform feature
> > > set\n");
> > > + }
> > > }
> >
> > Why dont we have some clean and robust PCI config space based
> > enumeration instead of this boot ID based thing?
>
> MRST has special PCI support that depends on having the platform
> identified already, so if you want to use something other than a
> flag here, maybe we could use an SFI string instead.
An SFI flag would be nicer than any PCI space method i guess - if it
can be accessed early enough.
Ingo
On Fri 2009-06-26 09:54:54, Jesse Barnes wrote:
> On Fri, 26 Jun 2009 18:32:42 +0200 (CEST)
> Thomas Gleixner <[email protected]> wrote:
>
> > On Fri, 26 Jun 2009, Ingo Molnar wrote:
> > > [ Although it is beyond me why ABP was done - why wasnt HPET good
> > > enough? HPET can do per CPU clockevents too and it's just as
> > > off-chip (and hence fundamentally slow) as ABP. ]
> >
> > Welcome to the wonderful world of embedded systems. Just have a peek
> > into arch/[arm/powerpc/mips] to see what's coming up to us with full
> > force. I would not be surprised when we see an x86 system sharing the
> > device driver for i2c or whatever with an ARM SoC in the foreseable
> > future.
>
> Ha, yeah I was just going to say "think embedded". ABP is a much
> simpler spec and programming interface than HPET, and since we were
> designing new custom silicon, it made sense to just do the simple
> thing, rather than butchering an existing spec, then making a partial
> HPET that looks like ABP anyway and forcing any future HPET updates to
> conform to the new standard (very similar reasoning to the ACPI vs SFI
> discussion btw). Hopefully the technologies we've come up with for
Very similary wrong, I'd say :-(. While you could have created
hpet-lite, where hpet-lite driver would work on hpet system, you went
and created something new.
And yes, SFI is similar disaster, you should just define subset of
acpi ('acpi-lite').
In the end, you are willing to use silicon for compatibility (arm
instruction set needs less transistors, right?) and wasting millions
of transistors, then try to save thousands with non-compatible
devices :-(.
Pavel
--
(english) http://www.livejournal.com/~pavelmachek
(cesky, pictures) http://atrey.karlin.mff.cuni.cz/~pavel/picture/horses/blog.html
On Tue, 30 Jun 2009 08:35:13 +0200
Pavel Machek <[email protected]> wrote:
> On Fri 2009-06-26 09:54:54, Jesse Barnes wrote:
> > On Fri, 26 Jun 2009 18:32:42 +0200 (CEST)
> > Thomas Gleixner <[email protected]> wrote:
> >
> > > On Fri, 26 Jun 2009, Ingo Molnar wrote:
> > > > [ Although it is beyond me why ABP was done - why wasnt HPET
> > > > good enough? HPET can do per CPU clockevents too and it's just
> > > > as off-chip (and hence fundamentally slow) as ABP. ]
> > >
> > > Welcome to the wonderful world of embedded systems. Just have a
> > > peek into arch/[arm/powerpc/mips] to see what's coming up to us
> > > with full force. I would not be surprised when we see an x86
> > > system sharing the device driver for i2c or whatever with an ARM
> > > SoC in the foreseable future.
> >
> > Ha, yeah I was just going to say "think embedded". ABP is a much
> > simpler spec and programming interface than HPET, and since we were
> > designing new custom silicon, it made sense to just do the simple
> > thing, rather than butchering an existing spec, then making a
> > partial HPET that looks like ABP anyway and forcing any future HPET
> > updates to conform to the new standard (very similar reasoning to
> > the ACPI vs SFI discussion btw). Hopefully the technologies we've
> > come up with for
>
> Very similary wrong, I'd say :-(. While you could have created
> hpet-lite, where hpet-lite driver would work on hpet system, you went
> and created something new.
>
> And yes, SFI is similar disaster, you should just define subset of
> acpi ('acpi-lite').
>
> In the end, you are willing to use silicon for compatibility (arm
> instruction set needs less transistors, right?) and wasting millions
> of transistors, then try to save thousands with non-compatible
> devices :-(.
You didn't address the essence of either argument; butchering an
existing spec and placing an extra burden on all future implementations
is a high price to pay, both in terms of complexity and cost.
But really these are moot points; this is how the platform works. I'd
like to see Linux run on it "out of the box" which means integrating
support for these features in a maintainable way. Hopefully no one
disagrees with that; after all Linux runs on much uglier platforms than
this.
--
Jesse Barnes, Intel Open Source Technology Center
* Jesse Barnes <[email protected]> wrote:
> On Tue, 30 Jun 2009 08:35:13 +0200
> Pavel Machek <[email protected]> wrote:
>
> > On Fri 2009-06-26 09:54:54, Jesse Barnes wrote:
> > > On Fri, 26 Jun 2009 18:32:42 +0200 (CEST)
> > > Thomas Gleixner <[email protected]> wrote:
> > >
> > > > On Fri, 26 Jun 2009, Ingo Molnar wrote:
> > > > > [ Although it is beyond me why ABP was done - why wasnt HPET
> > > > > good enough? HPET can do per CPU clockevents too and it's just
> > > > > as off-chip (and hence fundamentally slow) as ABP. ]
> > > >
> > > > Welcome to the wonderful world of embedded systems. Just have a
> > > > peek into arch/[arm/powerpc/mips] to see what's coming up to us
> > > > with full force. I would not be surprised when we see an x86
> > > > system sharing the device driver for i2c or whatever with an ARM
> > > > SoC in the foreseable future.
> > >
> > > Ha, yeah I was just going to say "think embedded". ABP is a much
> > > simpler spec and programming interface than HPET, and since we were
> > > designing new custom silicon, it made sense to just do the simple
> > > thing, rather than butchering an existing spec, then making a
> > > partial HPET that looks like ABP anyway and forcing any future HPET
> > > updates to conform to the new standard (very similar reasoning to
> > > the ACPI vs SFI discussion btw). Hopefully the technologies we've
> > > come up with for
> >
> > Very similary wrong, I'd say :-(. While you could have created
> > hpet-lite, where hpet-lite driver would work on hpet system, you went
> > and created something new.
> >
> > And yes, SFI is similar disaster, you should just define subset of
> > acpi ('acpi-lite').
> >
> > In the end, you are willing to use silicon for compatibility (arm
> > instruction set needs less transistors, right?) and wasting millions
> > of transistors, then try to save thousands with non-compatible
> > devices :-(.
>
> You didn't address the essence of either argument; butchering an
> existing spec and placing an extra burden on all future
> implementations is a high price to pay, both in terms of
> complexity and cost.
>
> But really these are moot points; this is how the platform works.
> I'd like to see Linux run on it "out of the box" which means
> integrating support for these features in a maintainable way.
> Hopefully no one disagrees with that; after all Linux runs on much
> uglier platforms than this.
I agree and that's fine.
Note that obviously ugliness increases the integration costs and
latency for Intel: all ugly bits have to be factored out cleanly,
have to be turned into isolated components and tucked away, so that
the ugliness does not spread.
If something is just plain 'standard', that makes it really easy to
merge. The less standard something is, the more trouble integration
becomes.
Also, i genuinely think that it's perfectly fine to iterate hardware
specs - to simplify and enhance it. And this is an area where Linux
is the strongest - we can support just about anything and can do it
quickly. I just wish that those variations were something to be
unconditionally proud of from an engineering POV ;-)
So for similar projects in the future - it's OK to be non-standard,
but the preferred direction should be something genuinely better
designed, or something that is just a _sub-set_ of the standard.
Supporting a sub-set is way easier than supporting something that is
different in some unanticipated way and needs an extension of
infrastructure.
The IO-APIC bits seems to be the latter in this case - i.e. they are
an "IO-APIC light" implementation in essence. And that shows
immediately: the patches arent all that horrible.
And SFI seems useful to me as well, not because it's a sub-set of
ACPI, but because i find it cleaner than ACPI.
The ABP timer is another matter - it's a completely separate
from-scratch thing that is dissimilar to everything that exists.
OTOH, timers are pretty separate entities just (still) not
abstracted out well enough on x86 yet.
Ingo