* [PATCH] x86: enable x2apic early at the first point
@ 2009-02-19 21:50 Yinghai Lu
2009-02-19 22:13 ` Suresh Siddha
2009-02-20 9:51 ` Ingo Molnar
0 siblings, 2 replies; 18+ messages in thread
From: Yinghai Lu @ 2009-02-19 21:50 UTC (permalink / raw)
To: Ingo Molnar, Thomas Gleixner, H. Peter Anvin, Andrew Morton,
Suresh Siddha
Cc: linux-kernel@vger.kernel.org
Impact: fix bug.
otherwise will get panic from early_acpi_boot_init()
also make disable_x2apic global, so could use it it x2_apic_xxx.c
and can get warning if preenabled system using nox2apic.
Signed-off-by: Yinghai Lu <yinghai@kernel.org>
---
arch/x86/kernel/apic/apic.c | 3 +--
arch/x86/kernel/apic/x2apic_cluster.c | 5 ++++-
arch/x86/kernel/apic/x2apic_phys.c | 5 ++++-
arch/x86/kernel/apic/x2apic_uv_x.c | 4 +++-
drivers/pci/dmar.c | 3 ++-
5 files changed, 14 insertions(+), 6 deletions(-)
Index: linux-2.6/arch/x86/kernel/apic/x2apic_cluster.c
===================================================================
--- linux-2.6.orig/arch/x86/kernel/apic/x2apic_cluster.c
+++ linux-2.6/arch/x86/kernel/apic/x2apic_cluster.c
@@ -14,8 +14,11 @@ DEFINE_PER_CPU(u32, x86_cpu_to_logical_a
static int x2apic_acpi_madt_oem_check(char *oem_id, char *oem_table_id)
{
- if (cpu_has_x2apic)
+ if (cpu_has_x2apic && !disable_x2apic) {
+ x2apic = 1;
+ enable_x2apic();
return 1;
+ }
return 0;
}
Index: linux-2.6/arch/x86/kernel/apic/x2apic_phys.c
===================================================================
--- linux-2.6.orig/arch/x86/kernel/apic/x2apic_phys.c
+++ linux-2.6/arch/x86/kernel/apic/x2apic_phys.c
@@ -21,8 +21,11 @@ early_param("x2apic_phys", set_x2apic_ph
static int x2apic_acpi_madt_oem_check(char *oem_id, char *oem_table_id)
{
- if (cpu_has_x2apic && x2apic_phys)
+ if (cpu_has_x2apic && !disable_x2apic && x2apic_phys) {
+ x2apic = 1;
+ enable_x2apic();
return 1;
+ }
return 0;
}
Index: linux-2.6/arch/x86/kernel/apic/x2apic_uv_x.c
===================================================================
--- linux-2.6.orig/arch/x86/kernel/apic/x2apic_uv_x.c
+++ linux-2.6/arch/x86/kernel/apic/x2apic_uv_x.c
@@ -41,8 +41,10 @@ static int uv_acpi_madt_oem_check(char *
uv_system_type = UV_LEGACY_APIC;
else if (!strcmp(oem_table_id, "UVX"))
uv_system_type = UV_X2APIC;
- else if (!strcmp(oem_table_id, "UVH")) {
+ else if (!strcmp(oem_table_id, "UVH") && !disable_x2apic) {
uv_system_type = UV_NON_UNIQUE_APIC;
+ x2apic = 1;
+ enable_x2apic();
return 1;
}
}
Index: linux-2.6/arch/x86/kernel/apic/apic.c
===================================================================
--- linux-2.6.orig/arch/x86/kernel/apic/apic.c
+++ linux-2.6/arch/x86/kernel/apic/apic.c
@@ -116,11 +116,10 @@ __setup("apicpmtimer", setup_apicpmtimer
int x2apic;
/* x2apic enabled before OS handover */
static int x2apic_preenabled;
-static int disable_x2apic;
+int disable_x2apic;
static __init int setup_nox2apic(char *str)
{
disable_x2apic = 1;
- setup_clear_cpu_cap(X86_FEATURE_X2APIC);
return 0;
}
early_param("nox2apic", setup_nox2apic);
Index: linux-2.6/drivers/pci/dmar.c
===================================================================
--- linux-2.6.orig/drivers/pci/dmar.c
+++ linux-2.6/drivers/pci/dmar.c
@@ -472,7 +472,8 @@ void __init detect_intel_iommu(void)
* is added, we will not need this any more.
*/
dmar = (struct acpi_table_dmar *) dmar_tbl;
- if (ret && cpu_has_x2apic && dmar->flags & 0x1)
+ if (ret && cpu_has_x2apic && !disable_x2apic &&
+ dmar->flags & 0x1)
printk(KERN_INFO
"Queued invalidation will be enabled to support "
"x2apic and Intr-remapping.\n");
^ permalink raw reply [flat|nested] 18+ messages in thread* Re: [PATCH] x86: enable x2apic early at the first point 2009-02-19 21:50 [PATCH] x86: enable x2apic early at the first point Yinghai Lu @ 2009-02-19 22:13 ` Suresh Siddha 2009-02-19 22:42 ` Yinghai Lu 2009-02-20 9:51 ` Ingo Molnar 1 sibling, 1 reply; 18+ messages in thread From: Suresh Siddha @ 2009-02-19 22:13 UTC (permalink / raw) To: Yinghai Lu Cc: Ingo Molnar, Thomas Gleixner, H. Peter Anvin, Andrew Morton, linux-kernel@vger.kernel.org On Thu, 2009-02-19 at 13:50 -0800, Yinghai Lu wrote: > Impact: fix bug. > > otherwise will get panic from early_acpi_boot_init() > also make disable_x2apic global, so could use it it x2_apic_xxx.c > and can get warning if preenabled system using nox2apic. > > Signed-off-by: Yinghai Lu <yinghai@kernel.org> > > --- > arch/x86/kernel/apic/apic.c | 3 +-- > arch/x86/kernel/apic/x2apic_cluster.c | 5 ++++- > arch/x86/kernel/apic/x2apic_phys.c | 5 ++++- > arch/x86/kernel/apic/x2apic_uv_x.c | 4 +++- > drivers/pci/dmar.c | 3 ++- > 5 files changed, 14 insertions(+), 6 deletions(-) > > Index: linux-2.6/arch/x86/kernel/apic/x2apic_cluster.c > =================================================================== > --- linux-2.6.orig/arch/x86/kernel/apic/x2apic_cluster.c > +++ linux-2.6/arch/x86/kernel/apic/x2apic_cluster.c > @@ -14,8 +14,11 @@ DEFINE_PER_CPU(u32, x86_cpu_to_logical_a > > static int x2apic_acpi_madt_oem_check(char *oem_id, char *oem_table_id) > { > - if (cpu_has_x2apic) > + if (cpu_has_x2apic && !disable_x2apic) { > + x2apic = 1; > + enable_x2apic(); Yinghai, I also ran into couple of x2apic issues on the latest tip and this issue is one of them. I don't like this patch mainly because we are enabling x2apic mode in the cpu, with out checking the intr-remapping support. We should fall back to xapic mode (in a clean fashion) if there is any issue with intr-remapping. So I am planning to post another patch to fix this early hang issue that we both encountered. thanks, suresh > return 1; > + } > > return 0; > } > Index: linux-2.6/arch/x86/kernel/apic/x2apic_phys.c > =================================================================== > --- linux-2.6.orig/arch/x86/kernel/apic/x2apic_phys.c > +++ linux-2.6/arch/x86/kernel/apic/x2apic_phys.c > @@ -21,8 +21,11 @@ early_param("x2apic_phys", set_x2apic_ph > > static int x2apic_acpi_madt_oem_check(char *oem_id, char *oem_table_id) > { > - if (cpu_has_x2apic && x2apic_phys) > + if (cpu_has_x2apic && !disable_x2apic && x2apic_phys) { > + x2apic = 1; > + enable_x2apic(); > return 1; > + } > > return 0; > } > Index: linux-2.6/arch/x86/kernel/apic/x2apic_uv_x.c > =================================================================== > --- linux-2.6.orig/arch/x86/kernel/apic/x2apic_uv_x.c > +++ linux-2.6/arch/x86/kernel/apic/x2apic_uv_x.c > @@ -41,8 +41,10 @@ static int uv_acpi_madt_oem_check(char * > uv_system_type = UV_LEGACY_APIC; > else if (!strcmp(oem_table_id, "UVX")) > uv_system_type = UV_X2APIC; > - else if (!strcmp(oem_table_id, "UVH")) { > + else if (!strcmp(oem_table_id, "UVH") && !disable_x2apic) { > uv_system_type = UV_NON_UNIQUE_APIC; > + x2apic = 1; > + enable_x2apic(); > return 1; > } > } > Index: linux-2.6/arch/x86/kernel/apic/apic.c > =================================================================== > --- linux-2.6.orig/arch/x86/kernel/apic/apic.c > +++ linux-2.6/arch/x86/kernel/apic/apic.c > @@ -116,11 +116,10 @@ __setup("apicpmtimer", setup_apicpmtimer > int x2apic; > /* x2apic enabled before OS handover */ > static int x2apic_preenabled; > -static int disable_x2apic; > +int disable_x2apic; > static __init int setup_nox2apic(char *str) > { > disable_x2apic = 1; > - setup_clear_cpu_cap(X86_FEATURE_X2APIC); > return 0; > } > early_param("nox2apic", setup_nox2apic); > Index: linux-2.6/drivers/pci/dmar.c > =================================================================== > --- linux-2.6.orig/drivers/pci/dmar.c > +++ linux-2.6/drivers/pci/dmar.c > @@ -472,7 +472,8 @@ void __init detect_intel_iommu(void) > * is added, we will not need this any more. > */ > dmar = (struct acpi_table_dmar *) dmar_tbl; > - if (ret && cpu_has_x2apic && dmar->flags & 0x1) > + if (ret && cpu_has_x2apic && !disable_x2apic && > + dmar->flags & 0x1) > printk(KERN_INFO > "Queued invalidation will be enabled to support " > "x2apic and Intr-remapping.\n"); ^ permalink raw reply [flat|nested] 18+ messages in thread
* Re: [PATCH] x86: enable x2apic early at the first point 2009-02-19 22:13 ` Suresh Siddha @ 2009-02-19 22:42 ` Yinghai Lu 2009-02-19 23:28 ` Suresh Siddha 0 siblings, 1 reply; 18+ messages in thread From: Yinghai Lu @ 2009-02-19 22:42 UTC (permalink / raw) To: Suresh Siddha Cc: Ingo Molnar, Thomas Gleixner, H. Peter Anvin, Andrew Morton, linux-kernel@vger.kernel.org Suresh Siddha wrote: > On Thu, 2009-02-19 at 13:50 -0800, Yinghai Lu wrote: >> Impact: fix bug. >> >> otherwise will get panic from early_acpi_boot_init() >> also make disable_x2apic global, so could use it it x2_apic_xxx.c >> and can get warning if preenabled system using nox2apic. >> >> Signed-off-by: Yinghai Lu <yinghai@kernel.org> >> >> --- >> arch/x86/kernel/apic/apic.c | 3 +-- >> arch/x86/kernel/apic/x2apic_cluster.c | 5 ++++- >> arch/x86/kernel/apic/x2apic_phys.c | 5 ++++- >> arch/x86/kernel/apic/x2apic_uv_x.c | 4 +++- >> drivers/pci/dmar.c | 3 ++- >> 5 files changed, 14 insertions(+), 6 deletions(-) >> >> Index: linux-2.6/arch/x86/kernel/apic/x2apic_cluster.c >> =================================================================== >> --- linux-2.6.orig/arch/x86/kernel/apic/x2apic_cluster.c >> +++ linux-2.6/arch/x86/kernel/apic/x2apic_cluster.c >> @@ -14,8 +14,11 @@ DEFINE_PER_CPU(u32, x86_cpu_to_logical_a >> >> static int x2apic_acpi_madt_oem_check(char *oem_id, char *oem_table_id) >> { >> - if (cpu_has_x2apic) >> + if (cpu_has_x2apic && !disable_x2apic) { >> + x2apic = 1; >> + enable_x2apic(); > > Yinghai, I also ran into couple of x2apic issues on the latest tip and > this issue is one of them. I don't like this patch mainly because we are > enabling x2apic mode in the cpu, with out checking the intr-remapping > support. We should fall back to xapic mode (in a clean fashion) if there > is any issue with intr-remapping. So I am planning to post another patch > to fix this early hang issue that we both encountered. Ingo want to decouple that x2apic and intr_remapping. it seems it does work with x2apic without intr_remapping in one of setup. YH ^ permalink raw reply [flat|nested] 18+ messages in thread
* Re: [PATCH] x86: enable x2apic early at the first point 2009-02-19 22:42 ` Yinghai Lu @ 2009-02-19 23:28 ` Suresh Siddha 2009-02-20 8:35 ` Ingo Molnar 0 siblings, 1 reply; 18+ messages in thread From: Suresh Siddha @ 2009-02-19 23:28 UTC (permalink / raw) To: Yinghai Lu Cc: Ingo Molnar, Thomas Gleixner, H. Peter Anvin, Andrew Morton, linux-kernel@vger.kernel.org, gleb On Thu, 2009-02-19 at 14:42 -0800, Yinghai Lu wrote: > Ingo want to decouple that x2apic and intr_remapping. > it seems it does work with x2apic without intr_remapping in one of setup. x2apic with out intr-remapping is not architectural. Even when we have < 255 logical cpu's, logical x2apic id's will be greater than 16 bits even on a single/two socket systems and this will break interrupt delivery. Please look at the x2apic logical destination mode definition in the SDM (Section 9.7.2.3 and 9.7.2.4 in my copy of SDM Vol3a) While it might work in certain configurations (for example, physical mode with < 8 bit apicids), it is not architectural and implementation dependent, which may break in future generations. And also, logical x2apic mode has more advantages compared to physical mode (like using lowest priority delivery mode etc). I will post couple of patches, which revert's Gleb's patch and another fix for the early boot failure issue, tomorrow. thanks, suresh ^ permalink raw reply [flat|nested] 18+ messages in thread
* Re: [PATCH] x86: enable x2apic early at the first point 2009-02-19 23:28 ` Suresh Siddha @ 2009-02-20 8:35 ` Ingo Molnar 2009-02-20 9:09 ` Gleb Natapov 0 siblings, 1 reply; 18+ messages in thread From: Ingo Molnar @ 2009-02-20 8:35 UTC (permalink / raw) To: Suresh Siddha Cc: Yinghai Lu, Thomas Gleixner, H. Peter Anvin, Andrew Morton, linux-kernel@vger.kernel.org, gleb * Suresh Siddha <suresh.b.siddha@intel.com> wrote: > On Thu, 2009-02-19 at 14:42 -0800, Yinghai Lu wrote: > > Ingo want to decouple that x2apic and intr_remapping. > > it seems it does work with x2apic without intr_remapping in one of setup. > > x2apic with out intr-remapping is not architectural. Even when > we have < 255 logical cpu's, logical x2apic id's will be > greater than 16 bits even on a single/two socket systems and > this will break interrupt delivery. Please look at the x2apic > logical destination mode definition in the SDM (Section > 9.7.2.3 and 9.7.2.4 in my copy of SDM Vol3a) > > While it might work in certain configurations (for example, > physical mode with < 8 bit apicids), it is not architectural > and implementation dependent, which may break in future > generations. > > And also, logical x2apic mode has more advantages compared to > physical mode (like using lowest priority delivery mode etc). > > I will post couple of patches, which revert's Gleb's patch and > another fix for the early boot failure issue, tomorrow. Instead of the revert, we can zap this patch from tip:x86/apic: d13d2c3: x86, apic: decouple x2apic from interrupt remapping But it will all need more cleanups. For example the mandatory coupling between x2apic and intr-remap is not clearly spelled out. Also, some of the code has also become rather ugly. For example could you please work on eliminating this #ifdef-fest: earth4:~/tip> grep INTR_REMAP arch/x86/kernel/apic/io_apic.c #ifdef CONFIG_INTR_REMAP #ifdef CONFIG_INTR_REMAP #ifdef CONFIG_INTR_REMAP #ifdef CONFIG_INTR_REMAP #ifdef CONFIG_INTR_REMAP #ifdef CONFIG_INTR_REMAP #ifdef CONFIG_INTR_REMAP #ifdef CONFIG_INTR_REMAP #ifdef CONFIG_INTR_REMAP #ifdef CONFIG_INTR_REMAP #ifdef CONFIG_INTR_REMAP #ifdef CONFIG_INTR_REMAP #ifdef CONFIG_INTR_REMAP #ifdef CONFIG_INTR_REMAP #ifdef CONFIG_INTR_REMAP #ifdef CONFIG_INTR_REMAP #ifdef CONFIG_INTR_REMAP #ifdef CONFIG_INTR_REMAP #ifdef CONFIG_INTR_REMAP for example all the #ifdefs around irq_remapped() look bogus to me - dmar.h already defines irq_remapped() to always-0 when !CONFIG_INTR_REMAP. Ingo ^ permalink raw reply [flat|nested] 18+ messages in thread
* Re: [PATCH] x86: enable x2apic early at the first point 2009-02-20 8:35 ` Ingo Molnar @ 2009-02-20 9:09 ` Gleb Natapov 2009-02-20 9:41 ` Ingo Molnar 0 siblings, 1 reply; 18+ messages in thread From: Gleb Natapov @ 2009-02-20 9:09 UTC (permalink / raw) To: Ingo Molnar Cc: Suresh Siddha, Yinghai Lu, Thomas Gleixner, H. Peter Anvin, Andrew Morton, linux-kernel@vger.kernel.org On Fri, Feb 20, 2009 at 09:35:28AM +0100, Ingo Molnar wrote: > > * Suresh Siddha <suresh.b.siddha@intel.com> wrote: > > > On Thu, 2009-02-19 at 14:42 -0800, Yinghai Lu wrote: > > > Ingo want to decouple that x2apic and intr_remapping. > > > it seems it does work with x2apic without intr_remapping in one of setup. > > > > x2apic with out intr-remapping is not architectural. Even when > > we have < 255 logical cpu's, logical x2apic id's will be > > greater than 16 bits even on a single/two socket systems and > > this will break interrupt delivery. Please look at the x2apic > > logical destination mode definition in the SDM (Section > > 9.7.2.3 and 9.7.2.4 in my copy of SDM Vol3a) > > This is what Intel doc says: It is likely that processor implementations may choose to support less than 16 bits of the cluster ID or less than 16-bits of the Logical ID in the Logical Destination Register. However system software should be agnostic to the number of bits implemented in the cluster ID and logical ID sub-fields. The x2APIC hardware initialization will ensure that the appropriately initialized logical x2APIC IDs are available to system software and reads of non-implemented bits return zero. KVM will implement 0 bits of cluster ID and will want to use x2apic without implementing IR. > > While it might work in certain configurations (for example, > > physical mode with < 8 bit apicids), it is not architectural > > and implementation dependent, which may break in future > > generations. > > It will not break in future generation of KVM though. > > And also, logical x2apic mode has more advantages compared to > > physical mode (like using lowest priority delivery mode etc). > > > > I will post couple of patches, which revert's Gleb's patch and > > another fix for the early boot failure issue, tomorrow. > If you'll revert my patch it will not be possible to use x2apic in KVM (at least without KVM implementing interrupt remapping which is unneeded otherwise) and x2apic interface is much better for vitalization. Instead of reverting the patch it will be better to add check if x2apic can be used without intr-remmaping (all CPUs belong to cluster 0) or allow enabling of x2apic without IR if running as a guest. -- Gleb. ^ permalink raw reply [flat|nested] 18+ messages in thread
* Re: [PATCH] x86: enable x2apic early at the first point 2009-02-20 9:09 ` Gleb Natapov @ 2009-02-20 9:41 ` Ingo Molnar 2009-02-20 10:58 ` Gleb Natapov 0 siblings, 1 reply; 18+ messages in thread From: Ingo Molnar @ 2009-02-20 9:41 UTC (permalink / raw) To: Gleb Natapov Cc: Suresh Siddha, Yinghai Lu, Thomas Gleixner, H. Peter Anvin, Andrew Morton, linux-kernel@vger.kernel.org * Gleb Natapov <gleb@redhat.com> wrote: > > > I will post couple of patches, which revert's Gleb's patch > > > and another fix for the early boot failure issue, > > > tomorrow. > > If you'll revert my patch it will not be possible to use > x2apic in KVM (at least without KVM implementing interrupt > remapping which is unneeded otherwise) and x2apic interface is > much better for vitalization. Instead of reverting the patch > it will be better to add check if x2apic can be used without > intr-remmaping (all CPUs belong to cluster 0) or allow > enabling of x2apic without IR if running as a guest. yep, that would be required - because your patch can break real systems right now. Mind sending me a fix for it? I've applied Yinghai's fix as well, so please base it on latest tip:master. Ingo ^ permalink raw reply [flat|nested] 18+ messages in thread
* Re: [PATCH] x86: enable x2apic early at the first point 2009-02-20 9:41 ` Ingo Molnar @ 2009-02-20 10:58 ` Gleb Natapov 2009-02-20 11:06 ` Ingo Molnar 0 siblings, 1 reply; 18+ messages in thread From: Gleb Natapov @ 2009-02-20 10:58 UTC (permalink / raw) To: Ingo Molnar Cc: Suresh Siddha, Yinghai Lu, Thomas Gleixner, H. Peter Anvin, Andrew Morton, linux-kernel@vger.kernel.org On Fri, Feb 20, 2009 at 10:41:23AM +0100, Ingo Molnar wrote: > > * Gleb Natapov <gleb@redhat.com> wrote: > > > > > I will post couple of patches, which revert's Gleb's patch > > > > and another fix for the early boot failure issue, > > > > tomorrow. > > > > If you'll revert my patch it will not be possible to use > > x2apic in KVM (at least without KVM implementing interrupt > > remapping which is unneeded otherwise) and x2apic interface is > > much better for vitalization. Instead of reverting the patch > > it will be better to add check if x2apic can be used without > > intr-remmaping (all CPUs belong to cluster 0) or allow > > enabling of x2apic without IR if running as a guest. > > yep, that would be required - because your patch can break real > systems right now. Mind sending me a fix for it? > > I've applied Yinghai's fix as well, so please base it on latest > tip:master. > OK. Will send next week. -- Gleb. ^ permalink raw reply [flat|nested] 18+ messages in thread
* Re: [PATCH] x86: enable x2apic early at the first point 2009-02-20 10:58 ` Gleb Natapov @ 2009-02-20 11:06 ` Ingo Molnar 2009-02-20 11:06 ` Gleb Natapov 2009-02-20 12:33 ` Gleb Natapov 0 siblings, 2 replies; 18+ messages in thread From: Ingo Molnar @ 2009-02-20 11:06 UTC (permalink / raw) To: Gleb Natapov Cc: Suresh Siddha, Yinghai Lu, Thomas Gleixner, H. Peter Anvin, Andrew Morton, linux-kernel@vger.kernel.org * Gleb Natapov <gleb@redhat.com> wrote: > On Fri, Feb 20, 2009 at 10:41:23AM +0100, Ingo Molnar wrote: > > > > * Gleb Natapov <gleb@redhat.com> wrote: > > > > > > > I will post couple of patches, which revert's Gleb's patch > > > > > and another fix for the early boot failure issue, > > > > > tomorrow. > > > > > > If you'll revert my patch it will not be possible to use > > > x2apic in KVM (at least without KVM implementing interrupt > > > remapping which is unneeded otherwise) and x2apic interface is > > > much better for vitalization. Instead of reverting the patch > > > it will be better to add check if x2apic can be used without > > > intr-remmaping (all CPUs belong to cluster 0) or allow > > > enabling of x2apic without IR if running as a guest. > > > > yep, that would be required - because your patch can break real > > systems right now. Mind sending me a fix for it? > > > > I've applied Yinghai's fix as well, so please base it on latest > > tip:master. > > > OK. Will send next week. ok, that's too long of a breakage window - i'll revert it then, please send a new version once you have it. Thanks, Ingo ^ permalink raw reply [flat|nested] 18+ messages in thread
* Re: [PATCH] x86: enable x2apic early at the first point 2009-02-20 11:06 ` Ingo Molnar @ 2009-02-20 11:06 ` Gleb Natapov 2009-02-20 12:33 ` Gleb Natapov 1 sibling, 0 replies; 18+ messages in thread From: Gleb Natapov @ 2009-02-20 11:06 UTC (permalink / raw) To: Ingo Molnar Cc: Suresh Siddha, Yinghai Lu, Thomas Gleixner, H. Peter Anvin, Andrew Morton, linux-kernel@vger.kernel.org On Fri, Feb 20, 2009 at 12:06:51PM +0100, Ingo Molnar wrote: > > * Gleb Natapov <gleb@redhat.com> wrote: > > > On Fri, Feb 20, 2009 at 10:41:23AM +0100, Ingo Molnar wrote: > > > > > > * Gleb Natapov <gleb@redhat.com> wrote: > > > > > > > > > I will post couple of patches, which revert's Gleb's patch > > > > > > and another fix for the early boot failure issue, > > > > > > tomorrow. > > > > > > > > If you'll revert my patch it will not be possible to use > > > > x2apic in KVM (at least without KVM implementing interrupt > > > > remapping which is unneeded otherwise) and x2apic interface is > > > > much better for vitalization. Instead of reverting the patch > > > > it will be better to add check if x2apic can be used without > > > > intr-remmaping (all CPUs belong to cluster 0) or allow > > > > enabling of x2apic without IR if running as a guest. > > > > > > yep, that would be required - because your patch can break real > > > systems right now. Mind sending me a fix for it? > > > > > > I've applied Yinghai's fix as well, so please base it on latest > > > tip:master. > > > > > OK. Will send next week. > > ok, that's too long of a breakage window - i'll revert it then, > please send a new version once you have it. > Deal. -- Gleb. ^ permalink raw reply [flat|nested] 18+ messages in thread
* Re: [PATCH] x86: enable x2apic early at the first point 2009-02-20 11:06 ` Ingo Molnar 2009-02-20 11:06 ` Gleb Natapov @ 2009-02-20 12:33 ` Gleb Natapov 2009-02-20 12:59 ` Ingo Molnar 1 sibling, 1 reply; 18+ messages in thread From: Gleb Natapov @ 2009-02-20 12:33 UTC (permalink / raw) To: Ingo Molnar Cc: Suresh Siddha, Yinghai Lu, Thomas Gleixner, H. Peter Anvin, Andrew Morton, linux-kernel@vger.kernel.org On Fri, Feb 20, 2009 at 12:06:51PM +0100, Ingo Molnar wrote: > > > > If you'll revert my patch it will not be possible to use > > > > x2apic in KVM (at least without KVM implementing interrupt > > > > remapping which is unneeded otherwise) and x2apic interface is > > > > much better for vitalization. Instead of reverting the patch > > > > it will be better to add check if x2apic can be used without > > > > intr-remmaping (all CPUs belong to cluster 0) or allow > > > > enabling of x2apic without IR if running as a guest. > > > > > > yep, that would be required - because your patch can break real > > > systems right now. Mind sending me a fix for it? > > > > > > I've applied Yinghai's fix as well, so please base it on latest > > > tip:master. > > > > > OK. Will send next week. > > ok, that's too long of a breakage window - i'll revert it then, > please send a new version once you have it. > What approach you actually prefer? Enable x2apic without intr-remapping only when running in KVM, or even if running on real HW but all CPUs are in cluster 0? If former then below is updated patch (tested only in KVM) if later then it's definitely for next week :) From: Gleb Natapov <gleb@redhat.com> Subject: x86, apic: decouple x2apic from interrupt remapping Currently x2apic is used only if interrupt remapping can be enabled, and although there is not real HW that has x2apic but does not have intr-remapping the decoupling is useful since we want to emulate x2apic interface in KVM (it is more vitalization friendly), but we don't want to emulate intr-remapping just for that. Attached patch enables x2apic when running in kvm guest even if intr-remapping cannot be enabled. Signed-off-by: Gleb Natapov <gleb@redhat.com> diff --git a/arch/x86/kernel/apic/apic.c b/arch/x86/kernel/apic/apic.c index c3acf90..98b9a0b 100644 --- a/arch/x86/kernel/apic/apic.c +++ b/arch/x86/kernel/apic/apic.c @@ -48,6 +48,7 @@ #include <asm/idle.h> #include <asm/mtrr.h> #include <asm/smp.h> +#include <asm/kvm_para.h> unsigned int num_processors; @@ -1296,10 +1297,44 @@ void enable_x2apic(void) } } -void __init enable_IR_x2apic(void) +static int __init enable_IR(void) { #ifdef CONFIG_INTR_REMAP int ret; + + ret = dmar_table_init(); + if (ret) { + pr_info("dmar_table_init() failed with %d:\n", ret); + return ret; + } + + ret = save_mask_IO_APIC_setup(); + if (ret) { + pr_info("Saving IO-APIC state failed: %d\n", ret); + return ret; + } + + ret = enable_intr_remapping(1); + if (ret) { + restore_IO_APIC_setup(); + return ret; + } + + reinit_intr_remapped_IO_APIC(x2apic_preenabled); + + return 0; +#else + if (!kvm_para_available()) { + local_irq_restore(flags); + panic("x2apic enabled prior OS handover," + " enable CONFIG_INTR_REMAP"); + } + return -ENODEV; +#endif +} + +void __init enable_IR_x2apic(void) +{ unsigned long flags; if (!cpu_has_x2apic) @@ -1320,72 +1355,33 @@ void __init enable_IR_x2apic(void) return; } - ret = dmar_table_init(); - if (ret) { - pr_info("dmar_table_init() failed with %d:\n", ret); - - if (x2apic_preenabled) - panic("x2apic enabled by bios. But IR enabling failed"); - else - pr_info("Not enabling x2apic,Intr-remapping\n"); - return; - } - local_irq_save(flags); mask_8259A(); - ret = save_mask_IO_APIC_setup(); - if (ret) { - pr_info("Saving IO-APIC state failed: %d\n", ret); - goto end; - } - - ret = enable_intr_remapping(1); - - if (ret && x2apic_preenabled) { - local_irq_restore(flags); - panic("x2apic enabled by bios. But IR enabling failed"); + if (enable_IR()) { + if (!kvm_para_available()) { + if (x2apic_preenabled) { + local_irq_restore(flags); + panic("x2apic enabled by bios. " + "But IR enabling failed"); + } + goto no_x2apic; + } + } else { + pr_info("Enabled interrupt remapping for x2apic\n"); } - if (ret) - goto end_restore; - if (!x2apic) { x2apic = 1; enable_x2apic(); } -end_restore: - if (ret) - /* - * IR enabling failed - */ - restore_IO_APIC_setup(); - else - reinit_intr_remapped_IO_APIC(x2apic_preenabled); - -end: +no_x2apic: unmask_8259A(); local_irq_restore(flags); - if (!ret) { - if (!x2apic_preenabled) - pr_info("Enabled x2apic and interrupt-remapping\n"); - else - pr_info("Enabled Interrupt-remapping\n"); - } else - pr_err("Failed to enable Interrupt-remapping and x2apic\n"); -#else - if (!cpu_has_x2apic) - return; - - if (x2apic_preenabled) - panic("x2apic enabled prior OS handover," - " enable CONFIG_INTR_REMAP"); - - pr_info("Enable CONFIG_INTR_REMAP for enabling intr-remapping " - " and x2apic\n"); -#endif + if (x2apic && !x2apic_preenabled) + pr_info("Enabled x2apic\n"); return; } diff --git a/arch/x86/kernel/apic/probe_64.c b/arch/x86/kernel/apic/probe_64.c index 70935dd..8f11f6d 100644 --- a/arch/x86/kernel/apic/probe_64.c +++ b/arch/x86/kernel/apic/probe_64.c @@ -22,6 +22,7 @@ #include <asm/apic.h> #include <asm/ipi.h> #include <asm/setup.h> +#include <asm/kvm_para.h> extern struct apic apic_flat; extern struct apic apic_physflat; @@ -51,7 +52,7 @@ void __init default_setup_apic_routing(void) { #ifdef CONFIG_X86_X2APIC if (apic == &apic_x2apic_phys || apic == &apic_x2apic_cluster) { - if (!intr_remapping_enabled) + if (!intr_remapping_enabled && !kvm_para_available()) apic = &apic_flat; } #endif diff --git a/arch/x86/kernel/apic/x2apic_cluster.c b/arch/x86/kernel/apic/x2apic_cluster.c index 4e39d9a..3f7df23 100644 --- a/arch/x86/kernel/apic/x2apic_cluster.c +++ b/arch/x86/kernel/apic/x2apic_cluster.c @@ -24,7 +24,10 @@ static int x2apic_acpi_madt_oem_check(char *oem_id, char *oem_table_id) static const struct cpumask *x2apic_target_cpus(void) { - return cpumask_of(0); + if (intr_remapping_enabled) + return cpumask_of(0); + else + return cpu_online_mask; } /* @@ -116,37 +119,46 @@ static int x2apic_apic_id_registered(void) static unsigned int x2apic_cpu_mask_to_apicid(const struct cpumask *cpumask) { - /* - * We're using fixed IRQ delivery, can only return one logical APIC ID. - * May as well be the first. - */ - int cpu = cpumask_first(cpumask); + if (intr_remapping_enabled) { + /* + * We're using fixed IRQ delivery, can only return one + * logical APIC ID. May as well be the first. + */ + int cpu = cpumask_first(cpumask); - if ((unsigned)cpu < nr_cpu_ids) - return per_cpu(x86_cpu_to_logical_apicid, cpu); - else - return BAD_APICID; + if ((unsigned)cpu < nr_cpu_ids) + return per_cpu(x86_cpu_to_logical_apicid, cpu); + else + return BAD_APICID; + } else + return cpumask_bits(cpumask)[0] & APIC_ALL_CPUS; } static unsigned int x2apic_cpu_mask_to_apicid_and(const struct cpumask *cpumask, const struct cpumask *andmask) { - int cpu; + if (intr_remapping_enabled) { + int cpu; - /* - * We're using fixed IRQ delivery, can only return one logical APIC ID. - * May as well be the first. - */ - for_each_cpu_and(cpu, cpumask, andmask) { - if (cpumask_test_cpu(cpu, cpu_online_mask)) - break; - } + /* + * We're using fixed IRQ delivery, can only return one + * logical APIC ID. May as well be the first. + */ + for_each_cpu_and(cpu, cpumask, andmask) { + if (cpumask_test_cpu(cpu, cpu_online_mask)) + break; + } - if (cpu < nr_cpu_ids) - return per_cpu(x86_cpu_to_logical_apicid, cpu); + if (cpu < nr_cpu_ids) + return per_cpu(x86_cpu_to_logical_apicid, cpu); - return BAD_APICID; + return BAD_APICID; + } else { + unsigned long mask1 = cpumask_bits(cpumask)[0] & APIC_ALL_CPUS; + unsigned long mask2 = cpumask_bits(andmask)[0] & APIC_ALL_CPUS; + return mask1 & mask2; + } } static unsigned int x2apic_cluster_phys_get_apic_id(unsigned long x) -- Gleb. ^ permalink raw reply related [flat|nested] 18+ messages in thread
* Re: [PATCH] x86: enable x2apic early at the first point 2009-02-20 12:33 ` Gleb Natapov @ 2009-02-20 12:59 ` Ingo Molnar 0 siblings, 0 replies; 18+ messages in thread From: Ingo Molnar @ 2009-02-20 12:59 UTC (permalink / raw) To: Gleb Natapov Cc: Suresh Siddha, Yinghai Lu, Thomas Gleixner, H. Peter Anvin, Andrew Morton, linux-kernel@vger.kernel.org * Gleb Natapov <gleb@redhat.com> wrote: > On Fri, Feb 20, 2009 at 12:06:51PM +0100, Ingo Molnar wrote: > > > > > If you'll revert my patch it will not be possible to use > > > > > x2apic in KVM (at least without KVM implementing interrupt > > > > > remapping which is unneeded otherwise) and x2apic interface is > > > > > much better for vitalization. Instead of reverting the patch > > > > > it will be better to add check if x2apic can be used without > > > > > intr-remmaping (all CPUs belong to cluster 0) or allow > > > > > enabling of x2apic without IR if running as a guest. > > > > > > > > yep, that would be required - because your patch can break real > > > > systems right now. Mind sending me a fix for it? > > > > > > > > I've applied Yinghai's fix as well, so please base it on latest > > > > tip:master. > > > > > > > OK. Will send next week. > > > > ok, that's too long of a breakage window - i'll revert it then, > > please send a new version once you have it. > > > > What approach you actually prefer? Enable x2apic without > intr-remapping only when running in KVM, or even if running on > real HW but all CPUs are in cluster 0? If former then below is > updated patch (tested only in KVM) if later then it's > definitely for next week :) The latter would be the ideal solution - i.e. next week :) x2apic is a rare feature, and the more exposure we give it the more tested it becomes. It's also obviously good for general code structure if core CPU features are separated from chipset/iommu features. Plus, x2apic accesses could actually be faster than UC accesses to the lapic, so whenever we can we should enter x2apic mode - even if intr-remap is not turned on (because not needed). Ingo ^ permalink raw reply [flat|nested] 18+ messages in thread
* Re: [PATCH] x86: enable x2apic early at the first point 2009-02-19 21:50 [PATCH] x86: enable x2apic early at the first point Yinghai Lu 2009-02-19 22:13 ` Suresh Siddha @ 2009-02-20 9:51 ` Ingo Molnar 2009-02-20 9:55 ` Ingo Molnar 1 sibling, 1 reply; 18+ messages in thread From: Ingo Molnar @ 2009-02-20 9:51 UTC (permalink / raw) To: Yinghai Lu Cc: Thomas Gleixner, H. Peter Anvin, Andrew Morton, Suresh Siddha, linux-kernel@vger.kernel.org * Yinghai Lu <yinghai@kernel.org> wrote: > Impact: fix bug. > > otherwise will get panic from early_acpi_boot_init() > also make disable_x2apic global, so could use it it x2_apic_xxx.c > and can get warning if preenabled system using nox2apic. > > Signed-off-by: Yinghai Lu <yinghai@kernel.org> > > --- > arch/x86/kernel/apic/apic.c | 3 +-- > arch/x86/kernel/apic/x2apic_cluster.c | 5 ++++- > arch/x86/kernel/apic/x2apic_phys.c | 5 ++++- > arch/x86/kernel/apic/x2apic_uv_x.c | 4 +++- > drivers/pci/dmar.c | 3 ++- > 5 files changed, 14 insertions(+), 6 deletions(-) I've applied it because it fixes a real bug, but this code really needs a cleanup. Look at the repeat patterns: > Index: linux-2.6/arch/x86/kernel/apic/x2apic_cluster.c > =================================================================== > --- linux-2.6.orig/arch/x86/kernel/apic/x2apic_cluster.c > +++ linux-2.6/arch/x86/kernel/apic/x2apic_cluster.c > @@ -14,8 +14,11 @@ DEFINE_PER_CPU(u32, x86_cpu_to_logical_a > > static int x2apic_acpi_madt_oem_check(char *oem_id, char *oem_table_id) > { > - if (cpu_has_x2apic) > + if (cpu_has_x2apic && !disable_x2apic) { > + x2apic = 1; > + enable_x2apic(); > return 1; > + } > > return 0; > } > Index: linux-2.6/arch/x86/kernel/apic/x2apic_phys.c > =================================================================== > --- linux-2.6.orig/arch/x86/kernel/apic/x2apic_phys.c > +++ linux-2.6/arch/x86/kernel/apic/x2apic_phys.c > @@ -21,8 +21,11 @@ early_param("x2apic_phys", set_x2apic_ph > > static int x2apic_acpi_madt_oem_check(char *oem_id, char *oem_table_id) > { > - if (cpu_has_x2apic && x2apic_phys) > + if (cpu_has_x2apic && !disable_x2apic && x2apic_phys) { > + x2apic = 1; > + enable_x2apic(); > return 1; > + } > > return 0; > } > Index: linux-2.6/arch/x86/kernel/apic/x2apic_uv_x.c > =================================================================== > --- linux-2.6.orig/arch/x86/kernel/apic/x2apic_uv_x.c > +++ linux-2.6/arch/x86/kernel/apic/x2apic_uv_x.c > @@ -41,8 +41,10 @@ static int uv_acpi_madt_oem_check(char * > uv_system_type = UV_LEGACY_APIC; > else if (!strcmp(oem_table_id, "UVX")) > uv_system_type = UV_X2APIC; > - else if (!strcmp(oem_table_id, "UVH")) { > + else if (!strcmp(oem_table_id, "UVH") && !disable_x2apic) { > uv_system_type = UV_NON_UNIQUE_APIC; > + x2apic = 1; > + enable_x2apic(); > return 1; > } > } Such repeat patterns with small variations are always the sign of an inefficient code structure. The clean approach would be to have one generic helper function concentrated into enable_x2apic(). So instead of: static int x2apic_acpi_madt_oem_check(char *oem_id, char *oem_table_id) { if (cpu_has_x2apic && !disable_x2apic && x2apic_phys) { x2apic = 1; enable_x2apic(); return 1; } return 0; } We'd have: static int x2apic_acpi_madt_oem_check(char *oem_id, char *oem_table_id) { if (!x2apic_phys) return 0; return x86_enable_x2apic(); } That way all the repeat and common functionality (and the return code logic) is concentrated into enable_x2apic(), and the x2apic_acpi_madt_oem_check() function has _only_ its own special check open-coded. (namely, whether physical ID delivery mode is forced off.) Okay? Ingo ^ permalink raw reply [flat|nested] 18+ messages in thread
* Re: [PATCH] x86: enable x2apic early at the first point 2009-02-20 9:51 ` Ingo Molnar @ 2009-02-20 9:55 ` Ingo Molnar 2009-02-21 22:23 ` Suresh Siddha 0 siblings, 1 reply; 18+ messages in thread From: Ingo Molnar @ 2009-02-20 9:55 UTC (permalink / raw) To: Yinghai Lu Cc: Thomas Gleixner, H. Peter Anvin, Andrew Morton, Suresh Siddha, linux-kernel@vger.kernel.org * Ingo Molnar <mingo@elte.hu> wrote: > > arch/x86/kernel/apic/apic.c | 3 +-- > > arch/x86/kernel/apic/x2apic_cluster.c | 5 ++++- > > arch/x86/kernel/apic/x2apic_phys.c | 5 ++++- > > arch/x86/kernel/apic/x2apic_uv_x.c | 4 +++- > > drivers/pci/dmar.c | 3 ++- > > 5 files changed, 14 insertions(+), 6 deletions(-) > > I've applied it because it fixes a real bug, but this code > really needs a cleanup. Look at the repeat patterns: unapplied it again as it breaks the build: arch/x86/kernel/apic/x2apic_cluster.c: In function 'x2apic_acpi_madt_oem_check': arch/x86/kernel/apic/x2apic_cluster.c:17: error: 'disable_x2apic' undeclared (first use in this function) arch/x86/kernel/apic/x2apic_cluster.c:17: error: (Each undeclared identifier is reported only once arch/x86/kernel/apic/x2apic_cluster.c:17: error: for each function it appears in.) so please resend the fixed and cleaned up version. Thanks, Ingo ^ permalink raw reply [flat|nested] 18+ messages in thread
* Re: [PATCH] x86: enable x2apic early at the first point 2009-02-20 9:55 ` Ingo Molnar @ 2009-02-21 22:23 ` Suresh Siddha 2009-02-21 22:43 ` Yinghai Lu 2009-02-22 17:21 ` Ingo Molnar 0 siblings, 2 replies; 18+ messages in thread From: Suresh Siddha @ 2009-02-21 22:23 UTC (permalink / raw) To: Ingo Molnar Cc: Yinghai Lu, Thomas Gleixner, H. Peter Anvin, Andrew Morton, linux-kernel@vger.kernel.org On Fri, 2009-02-20 at 01:55 -0800, Ingo Molnar wrote: > * Ingo Molnar <mingo@elte.hu> wrote: > > > > arch/x86/kernel/apic/apic.c | 3 +-- > > > arch/x86/kernel/apic/x2apic_cluster.c | 5 ++++- > > > arch/x86/kernel/apic/x2apic_phys.c | 5 ++++- > > > arch/x86/kernel/apic/x2apic_uv_x.c | 4 +++- > > > drivers/pci/dmar.c | 3 ++- > > > 5 files changed, 14 insertions(+), 6 deletions(-) > > > > I've applied it because it fixes a real bug, but this code > > really needs a cleanup. Look at the repeat patterns: > > unapplied it again as it breaks the build: > > arch/x86/kernel/apic/x2apic_cluster.c: In function 'x2apic_acpi_madt_oem_check': > arch/x86/kernel/apic/x2apic_cluster.c:17: error: 'disable_x2apic' undeclared (first use in this function) > arch/x86/kernel/apic/x2apic_cluster.c:17: error: (Each undeclared identifier is reported only once > arch/x86/kernel/apic/x2apic_cluster.c:17: error: for each function it appears in.) > > so please resend the fixed and cleaned up version. From: Suresh Siddha <suresh.b.siddha@intel.com> Subject: x86: select x2apic ops in early apic probe only if x2apic mode is enabled If BIOS hands over the control to OS in legacy xapic mode, select legacy xapic related ops in the early apic probe and shift to x2apic ops later in the boot sequence, only after enabling x2apic mode. If BIOS hands over the control in x2apic mode, select x2apic related ops in the early apic probe. This fixes the early boot panic, where we were selecting x2apic ops, while the cpu is still in legacy xapic mode. Signed-off-by: Suresh Siddha <suresh.b.siddha@intel.com> --- Index: tip/arch/x86/include/asm/apic.h =================================================================== --- tip/arch/x86/include/asm/apic.h.orig +++ tip/arch/x86/include/asm/apic.h @@ -146,7 +146,7 @@ static inline u64 native_x2apic_icr_read return val; } -extern int x2apic; +extern int x2apic, x2apic_phys; extern void check_x2apic(void); extern void enable_x2apic(void); extern void enable_IR_x2apic(void); Index: tip/arch/x86/kernel/apic/probe_64.c =================================================================== --- tip/arch/x86/kernel/apic/probe_64.c.orig +++ tip/arch/x86/kernel/apic/probe_64.c @@ -50,9 +50,16 @@ static struct apic *apic_probe[] __initd void __init default_setup_apic_routing(void) { #ifdef CONFIG_X86_X2APIC - if (apic == &apic_x2apic_phys || apic == &apic_x2apic_cluster) { - if (!intr_remapping_enabled) - apic = &apic_flat; + if (x2apic && (apic != &apic_x2apic_phys && +#ifdef CONFIG_X86_UV + apic != &apic_x2apic_uv_x && +#endif + apic != &apic_x2apic_cluster)) { + if (x2apic_phys) + apic = &apic_x2apic_phys; + else + apic = &apic_x2apic_cluster; + printk(KERN_INFO "Setting APIC routing to %s\n", apic->name); } #endif Index: tip/arch/x86/kernel/apic/x2apic_cluster.c =================================================================== --- tip/arch/x86/kernel/apic/x2apic_cluster.c.orig +++ tip/arch/x86/kernel/apic/x2apic_cluster.c @@ -14,10 +14,7 @@ DEFINE_PER_CPU(u32, x86_cpu_to_logical_a static int x2apic_acpi_madt_oem_check(char *oem_id, char *oem_table_id) { - if (cpu_has_x2apic) - return 1; - - return 0; + return x2apic_enabled(); } /* Start with all IRQs pointing to boot CPU. IRQ balancing will shift them. */ Index: tip/arch/x86/kernel/apic/x2apic_phys.c =================================================================== --- tip/arch/x86/kernel/apic/x2apic_phys.c.orig +++ tip/arch/x86/kernel/apic/x2apic_phys.c @@ -10,7 +10,7 @@ #include <asm/apic.h> #include <asm/ipi.h> -static int x2apic_phys; +int x2apic_phys; static int set_x2apic_phys_mode(char *arg) { @@ -21,10 +21,10 @@ early_param("x2apic_phys", set_x2apic_ph static int x2apic_acpi_madt_oem_check(char *oem_id, char *oem_table_id) { - if (cpu_has_x2apic && x2apic_phys) - return 1; - - return 0; + if (x2apic_phys) + return x2apic_enabled(); + else + return 0; } /* Start with all IRQs pointing to boot CPU. IRQ balancing will shift them. */ Index: tip/arch/x86/kernel/apic/apic.c =================================================================== --- tip/arch/x86/kernel/apic/apic.c.orig +++ tip/arch/x86/kernel/apic/apic.c @@ -1269,14 +1269,7 @@ void __cpuinit end_local_APIC_setup(void #ifdef CONFIG_X86_X2APIC void check_x2apic(void) { - int msr, msr2; - - if (!cpu_has_x2apic) - return; - - rdmsr(MSR_IA32_APICBASE, msr, msr2); - - if (msr & X2APIC_ENABLE) { + if (x2apic_enabled()) { pr_info("x2apic enabled by BIOS, switching to x2apic ops\n"); x2apic_preenabled = x2apic = 1; } ^ permalink raw reply [flat|nested] 18+ messages in thread
* Re: [PATCH] x86: enable x2apic early at the first point 2009-02-21 22:23 ` Suresh Siddha @ 2009-02-21 22:43 ` Yinghai Lu 2009-02-21 23:33 ` Suresh Siddha 2009-02-22 17:21 ` Ingo Molnar 1 sibling, 1 reply; 18+ messages in thread From: Yinghai Lu @ 2009-02-21 22:43 UTC (permalink / raw) To: Suresh Siddha Cc: Ingo Molnar, Thomas Gleixner, H. Peter Anvin, Andrew Morton, linux-kernel@vger.kernel.org Suresh Siddha wrote: > On Fri, 2009-02-20 at 01:55 -0800, Ingo Molnar wrote: >> * Ingo Molnar <mingo@elte.hu> wrote: >> >>>> arch/x86/kernel/apic/apic.c | 3 +-- >>>> arch/x86/kernel/apic/x2apic_cluster.c | 5 ++++- >>>> arch/x86/kernel/apic/x2apic_phys.c | 5 ++++- >>>> arch/x86/kernel/apic/x2apic_uv_x.c | 4 +++- >>>> drivers/pci/dmar.c | 3 ++- >>>> 5 files changed, 14 insertions(+), 6 deletions(-) >>> I've applied it because it fixes a real bug, but this code >>> really needs a cleanup. Look at the repeat patterns: >> unapplied it again as it breaks the build: >> >> arch/x86/kernel/apic/x2apic_cluster.c: In function 'x2apic_acpi_madt_oem_check': >> arch/x86/kernel/apic/x2apic_cluster.c:17: error: 'disable_x2apic' undeclared (first use in this function) >> arch/x86/kernel/apic/x2apic_cluster.c:17: error: (Each undeclared identifier is reported only once >> arch/x86/kernel/apic/x2apic_cluster.c:17: error: for each function it appears in.) >> >> so please resend the fixed and cleaned up version. > > From: Suresh Siddha <suresh.b.siddha@intel.com> > Subject: x86: select x2apic ops in early apic probe only if x2apic mode is enabled > > If BIOS hands over the control to OS in legacy xapic mode, select legacy xapic > related ops in the early apic probe and shift to x2apic ops later in the boot > sequence, only after enabling x2apic mode. > > If BIOS hands over the control in x2apic mode, select x2apic related ops > in the early apic probe. > > This fixes the early boot panic, where we were selecting x2apic ops, > while the cpu is still in legacy xapic mode. good, other than that. for x2apic preenabled system, when nox2apic is used, cpu_has_x2apic will be cleared, apic will be xapic phys_flat or flat. is that expected? should 1. ignore nox2apic 2. or try to disable x2apic? YH ^ permalink raw reply [flat|nested] 18+ messages in thread
* Re: [PATCH] x86: enable x2apic early at the first point 2009-02-21 22:43 ` Yinghai Lu @ 2009-02-21 23:33 ` Suresh Siddha 0 siblings, 0 replies; 18+ messages in thread From: Suresh Siddha @ 2009-02-21 23:33 UTC (permalink / raw) To: Yinghai Lu Cc: Siddha, Suresh B, Ingo Molnar, Thomas Gleixner, H. Peter Anvin, Andrew Morton, linux-kernel@vger.kernel.org On Sat, Feb 21, 2009 at 02:43:46PM -0800, Yinghai Lu wrote: > for x2apic preenabled system, > when nox2apic is used, cpu_has_x2apic will be cleared, apic will be xapic phys_flat or flat. > is that expected? > > should > 1. ignore nox2apic > 2. or try to disable x2apic? This scenario might be useful for debug purposes? But it might not be simple/straight fwd in OS to implement this, as we need to do two things. 1. Go back to xapic mode using the state transition diagram in SDM. 2. And also, we need to disable the interrupt-remapping setup by the bios, so that chipset and cpu's are in same mode. If BIOS has enabled x2apic, it is for a reason (mostly platform has more logical cpus and hence need x2apic to brinup all the AP's etc). And typically other than very high end platforms, I don't expect bios to turn on x2apic. thanks, suresh ^ permalink raw reply [flat|nested] 18+ messages in thread
* Re: [PATCH] x86: enable x2apic early at the first point 2009-02-21 22:23 ` Suresh Siddha 2009-02-21 22:43 ` Yinghai Lu @ 2009-02-22 17:21 ` Ingo Molnar 1 sibling, 0 replies; 18+ messages in thread From: Ingo Molnar @ 2009-02-22 17:21 UTC (permalink / raw) To: Suresh Siddha Cc: Yinghai Lu, Thomas Gleixner, H. Peter Anvin, Andrew Morton, linux-kernel@vger.kernel.org * Suresh Siddha <suresh.b.siddha@intel.com> wrote: > On Fri, 2009-02-20 at 01:55 -0800, Ingo Molnar wrote: > > * Ingo Molnar <mingo@elte.hu> wrote: > > > > > > arch/x86/kernel/apic/apic.c | 3 +-- > > > > arch/x86/kernel/apic/x2apic_cluster.c | 5 ++++- > > > > arch/x86/kernel/apic/x2apic_phys.c | 5 ++++- > > > > arch/x86/kernel/apic/x2apic_uv_x.c | 4 +++- > > > > drivers/pci/dmar.c | 3 ++- > > > > 5 files changed, 14 insertions(+), 6 deletions(-) > > > > > > I've applied it because it fixes a real bug, but this code > > > really needs a cleanup. Look at the repeat patterns: > > > > unapplied it again as it breaks the build: > > > > arch/x86/kernel/apic/x2apic_cluster.c: In function 'x2apic_acpi_madt_oem_check': > > arch/x86/kernel/apic/x2apic_cluster.c:17: error: 'disable_x2apic' undeclared (first use in this function) > > arch/x86/kernel/apic/x2apic_cluster.c:17: error: (Each undeclared identifier is reported only once > > arch/x86/kernel/apic/x2apic_cluster.c:17: error: for each function it appears in.) > > > > so please resend the fixed and cleaned up version. > > From: Suresh Siddha <suresh.b.siddha@intel.com> > Subject: x86: select x2apic ops in early apic probe only if x2apic mode is enabled Applied to tip:x86/apic, thanks Suresh! Ingo ^ permalink raw reply [flat|nested] 18+ messages in thread
end of thread, other threads:[~2009-02-22 17:21 UTC | newest] Thread overview: 18+ messages (download: mbox.gz follow: Atom feed -- links below jump to the message on this page -- 2009-02-19 21:50 [PATCH] x86: enable x2apic early at the first point Yinghai Lu 2009-02-19 22:13 ` Suresh Siddha 2009-02-19 22:42 ` Yinghai Lu 2009-02-19 23:28 ` Suresh Siddha 2009-02-20 8:35 ` Ingo Molnar 2009-02-20 9:09 ` Gleb Natapov 2009-02-20 9:41 ` Ingo Molnar 2009-02-20 10:58 ` Gleb Natapov 2009-02-20 11:06 ` Ingo Molnar 2009-02-20 11:06 ` Gleb Natapov 2009-02-20 12:33 ` Gleb Natapov 2009-02-20 12:59 ` Ingo Molnar 2009-02-20 9:51 ` Ingo Molnar 2009-02-20 9:55 ` Ingo Molnar 2009-02-21 22:23 ` Suresh Siddha 2009-02-21 22:43 ` Yinghai Lu 2009-02-21 23:33 ` Suresh Siddha 2009-02-22 17:21 ` Ingo Molnar
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox