[PATCH] [0/4] More late merge bug fixes for 2.6.23

[PATCH] [0/4] More late merge bug fixes for 2.6.23

[Andi Kleen]

More late bug fixe for 2.6.23.

-Andi

[PATCH] [1/4] x86_64: Fail dma_alloc_coherent on dma less devices

[Andi Kleen]

This should fix an oops with PCMCIA PATA devices

http://bugzilla.kernel.org/show_bug.cgi?id=8424

This is not a full fix for the problem, but probably
still the right thing to do.

Signed-off-by: Andi Kleen <ak@suse.de>

---
 arch/x86_64/kernel/pci-dma.c |    4 ++++
 1 file changed, 4 insertions(+)

Index: linux/arch/x86_64/kernel/pci-dma.c
===================================================================
--- linux.orig/arch/x86_64/kernel/pci-dma.c
+++ linux/arch/x86_64/kernel/pci-dma.c
@@ -82,6 +82,10 @@ dma_alloc_coherent(struct device *dev, s
 	if (dma_mask == 0)
 		dma_mask = DMA_32BIT_MASK;
 
+	/* Device not DMA able */
+	if (dev->dma_mask == NULL)
+		return NULL;
+
 	/* Don't invoke OOM killer */
 	gfp |= __GFP_NORETRY;
 

Re: [PATCH] [1/4] x86_64: Fail dma_alloc_coherent on dma less devices

[Linus Torvalds]

Hmm. I think this is wrong. 

Why? Because the regular 32-bit x86 code does this all completely 
differently, and doesn't use dma_mask at all. Instead, it _only_ uses 
dev->coherent_dma_mask (which, considering the name of the function, 
would seem to make sense).

Considering that the oops comes from this:

        /* Kludge to make it bug-to-bug compatible with i386. i386
           uses the normal dma_mask for alloc_coherent. */
        dma_mask &= *dev->dma_mask;

and that that code is *old*, and comes from when this file was called 
arch/x86_64/kernel/pci-gart.c, and the comment doesn't seem to even be 
correct any more, I really think the proper fix is likely to just *remove* 
that kludge that causes the oops entirely.

Anyway, I'll apply the patch, because clearly it's not going to make 
things *worse* (it does avoid the oops that we get now), but I don't think 
it's really even "probably still the right thing to do". I really think we 
should just remove the line that causes the oops instead, but that might 
change behaviour for non-oopsing cases, so I'm not ready to do that at 
this point.

Hmm? Who feels in charge of the DMA mapping stuff? Muli? James? Anybody?

		Linus

On Wed, 15 Aug 2007, Andi Kleen wrote:
> 
> This should fix an oops with PCMCIA PATA devices
> 
> http://bugzilla.kernel.org/show_bug.cgi?id=8424
> 
> This is not a full fix for the problem, but probably
> still the right thing to do.
> 
> Signed-off-by: Andi Kleen <ak@suse.de>
> 
> ---
>  arch/x86_64/kernel/pci-dma.c |    4 ++++
>  1 file changed, 4 insertions(+)
> 
> Index: linux/arch/x86_64/kernel/pci-dma.c
> ===================================================================
> --- linux.orig/arch/x86_64/kernel/pci-dma.c
> +++ linux/arch/x86_64/kernel/pci-dma.c
> @@ -82,6 +82,10 @@ dma_alloc_coherent(struct device *dev, s
>  	if (dma_mask == 0)
>  		dma_mask = DMA_32BIT_MASK;
>  
> +	/* Device not DMA able */
> +	if (dev->dma_mask == NULL)
> +		return NULL;
> +
>  	/* Don't invoke OOM killer */
>  	gfp |= __GFP_NORETRY;
>  
> 

Re: [PATCH] [1/4] x86_64: Fail dma_alloc_coherent on dma less devices

[Andi Kleen]

On Saturday 18 August 2007 19:22:18 Linus Torvalds wrote:
> 
> Hmm. I think this is wrong. 
> 
> Why? Because the regular 32-bit x86 code does this all completely 
> differently, and doesn't use dma_mask at all. Instead, it _only_ uses 
> dev->coherent_dma_mask (which, considering the name of the function, 
> would seem to make sense).

Yes, see my discussion with Alan. Likely this needs to be handled
in the caller to really fix the problem (pata_pcmcia oopsing when
it passes a DMA incapable pcmcia device to dma_alloc_coherent) 

Another possible fix proposed by James was to give the pcmcia
devices a dma_mask of 0 which would also work.

> Considering that the oops comes from this:
> 
>         /* Kludge to make it bug-to-bug compatible with i386. i386
>            uses the normal dma_mask for alloc_coherent. */
>         dma_mask &= *dev->dma_mask;

> 
> and that that code is *old*, and comes from when this file was called 
> arch/x86_64/kernel/pci-gart.c, and the comment doesn't seem to even be 

It might be outdated or it might now. The kludge was needed for Alsa because old
i386 ignored the consistent mask and they didn't always set it correctly, but that 
should be obsolete now? I'm not quite sure because sound devices
are not always well tested on large memory systems which are the only
ones who show this problem. Takashi, do you know if all alsa drivers
set consistent mask correctly now?

But I didn't want to touch that that late -- can do it for .24.

Still failing is probably correct in this case so I included the patch.

-Andi

Re: [PATCH] [1/4] x86_64: Fail dma_alloc_coherent on dma less devices

[Takashi Iwai]

At Sat, 18 Aug 2007 21:03:25 +0200,
Andi Kleen wrote:
> 
> > Considering that the oops comes from this:
> > 
> >         /* Kludge to make it bug-to-bug compatible with i386. i386
> >            uses the normal dma_mask for alloc_coherent. */
> >         dma_mask &= *dev->dma_mask;
> 
> > 
> > and that that code is *old*, and comes from when this file was called 
> > arch/x86_64/kernel/pci-gart.c, and the comment doesn't seem to even be 
> 
> It might be outdated or it might now. The kludge was needed for Alsa because old
> i386 ignored the consistent mask and they didn't always set it correctly, but that 
> should be obsolete now? I'm not quite sure because sound devices
> are not always well tested on large memory systems which are the only
> ones who show this problem. Takashi, do you know if all alsa drivers
> set consistent mask correctly now?

Yes, ALSA driver set both masks from the very beginning :)

There are some OSS drivers still calling only pci_set_dma_mask().
Through a quick look, maybe only trident.c needs a fix because it uses
a mask < 32bit for ALI devices.  Others use 32bit DMA mask.


Takashi

[PATCH] [2/4] x86_64: Fix to keep watchdog disabled by default for i386/x86_64

[Andi Kleen]

From: Daniel Gollub <dgollub@suse.de>
Fixed wrong expression which enabled watchdogs even if nmi_watchdog kernel 
parameter wasn't set. This regression got slightly introduced with commit 
b7471c6da94d30d3deadc55986cc38d1ff57f9ca.

Introduced NMI_DISABLED (-1) which allows to switch the value of NMI_DEFAULT 
without breaking the APIC NMI watchdog code (again).

Fixes:
https://bugzilla.novell.com/show_bug.cgi?id=298084
http://bugzilla.kernel.org/show_bug.cgi?id=7839
And likely some more nmi_watchdog=0 related issues.

Signed-off-by: Daniel Gollub <dgollub@suse.de>
Signed-off-by: Andi Kleen <ak@suse.de>

---
Index: linux/arch/i386/kernel/apic.c
===================================================================
--- linux.orig/arch/i386/kernel/apic.c
+++ linux/arch/i386/kernel/apic.c
@@ -1085,7 +1085,7 @@ static int __init detect_init_APIC (void
 	if (l & MSR_IA32_APICBASE_ENABLE)
 		mp_lapic_addr = l & MSR_IA32_APICBASE_BASE;
 
-	if (nmi_watchdog != NMI_NONE)
+	if (nmi_watchdog != NMI_NONE && nmi_watchdog != NMI_DISABLED)
 		nmi_watchdog = NMI_LOCAL_APIC;
 
 	printk(KERN_INFO "Found and enabled local APIC!\n");
Index: linux/arch/i386/kernel/nmi.c
===================================================================
--- linux.orig/arch/i386/kernel/nmi.c
+++ linux/arch/i386/kernel/nmi.c
@@ -77,7 +77,7 @@ static int __init check_nmi_watchdog(voi
 	unsigned int *prev_nmi_count;
 	int cpu;
 
-	if ((nmi_watchdog == NMI_NONE) || (nmi_watchdog == NMI_DEFAULT))
+	if ((nmi_watchdog == NMI_NONE) || (nmi_watchdog == NMI_DISABLED))
 		return 0;
 
 	if (!atomic_read(&nmi_active))
@@ -424,7 +424,7 @@ int proc_nmi_enabled(struct ctl_table *t
 	if (!!old_state == !!nmi_watchdog_enabled)
 		return 0;
 
-	if (atomic_read(&nmi_active) < 0) {
+	if (atomic_read(&nmi_active) < 0 || nmi_watchdog == NMI_DISABLED) {
 		printk( KERN_WARNING "NMI watchdog is permanently disabled\n");
 		return -EIO;
 	}
Index: linux/arch/x86_64/kernel/nmi.c
===================================================================
--- linux.orig/arch/x86_64/kernel/nmi.c
+++ linux/arch/x86_64/kernel/nmi.c
@@ -85,7 +85,7 @@ int __init check_nmi_watchdog (void)
 	int *counts;
 	int cpu;
 
-	if ((nmi_watchdog == NMI_NONE) || (nmi_watchdog == NMI_DEFAULT))
+	if ((nmi_watchdog == NMI_NONE) || (nmi_watchdog == NMI_DISABLED)) 
 		return 0;
 
 	if (!atomic_read(&nmi_active))
@@ -442,7 +442,7 @@ int proc_nmi_enabled(struct ctl_table *t
 	if (!!old_state == !!nmi_watchdog_enabled)
 		return 0;
 
-	if (atomic_read(&nmi_active) < 0) {
+	if (atomic_read(&nmi_active) < 0 || nmi_watchdog == NMI_DISABLED) {
 		printk( KERN_WARNING "NMI watchdog is permanently disabled\n");
 		return -EIO;
 	}
Index: linux/include/asm-i386/nmi.h
===================================================================
--- linux.orig/include/asm-i386/nmi.h
+++ linux/include/asm-i386/nmi.h
@@ -33,11 +33,12 @@ extern int nmi_watchdog_tick (struct pt_
 
 extern atomic_t nmi_active;
 extern unsigned int nmi_watchdog;
-#define NMI_DEFAULT     -1
+#define NMI_DISABLED    -1
 #define NMI_NONE	0
 #define NMI_IO_APIC	1
 #define NMI_LOCAL_APIC	2
 #define NMI_INVALID	3
+#define NMI_DEFAULT	NMI_DISABLED
 
 struct ctl_table;
 struct file;
Index: linux/include/asm-x86_64/nmi.h
===================================================================
--- linux.orig/include/asm-x86_64/nmi.h
+++ linux/include/asm-x86_64/nmi.h
@@ -64,11 +64,12 @@ extern int setup_nmi_watchdog(char *);
 
 extern atomic_t nmi_active;
 extern unsigned int nmi_watchdog;
-#define NMI_DEFAULT	-1
+#define NMI_DISABLED    -1
 #define NMI_NONE	0
 #define NMI_IO_APIC	1
 #define NMI_LOCAL_APIC	2
 #define NMI_INVALID	3
+#define NMI_DEFAULT	NMI_DISABLED
 
 struct ctl_table;
 struct file;

[PATCH] [3/4] x86_64: Change PMDS invocation to single macro

[Andi Kleen]

Very old binutils (2.12.90...) seem to have trouble with newlines
in assembler macro invocation. They put them into the resulting
argument expansion. In this case this lead to a parse error because
a .rept expression ended up spread over multiple lines. Change the PMDS() 
invocation to a single line.

Signed-off-by: Andi Kleen <ak@suse.de>

Index: linux/arch/x86_64/kernel/head.S
===================================================================
--- linux.orig/arch/x86_64/kernel/head.S
+++ linux/arch/x86_64/kernel/head.S
@@ -345,8 +345,7 @@ NEXT_PAGE(level2_kernel_pgt)
 	/* 40MB kernel mapping. The kernel code cannot be bigger than that.
 	   When you change this change KERNEL_TEXT_SIZE in page.h too. */
 	/* (2^48-(2*1024*1024*1024)-((2^39)*511)-((2^30)*510)) = 0 */
-	PMDS(0x0000000000000000, __PAGE_KERNEL_LARGE_EXEC|_PAGE_GLOBAL,
-		KERNEL_TEXT_SIZE/PMD_SIZE)
+	PMDS(0x0000000000000000, __PAGE_KERNEL_LARGE_EXEC|_PAGE_GLOBAL, KERNEL_TEXT_SIZE/PMD_SIZE)
 	/* Module mapping starts here */
 	.fill	(PTRS_PER_PMD - (KERNEL_TEXT_SIZE/PMD_SIZE)),8,0
 

Re: [PATCH] [3/4] x86_64: Change PMDS invocation to single macro

[Clemens Koller]

Andi Kleen schrieb:
> Very old binutils (2.12.90...) seem to have trouble with newlines
> in assembler macro invocation. They put them into the resulting
> argument expansion. In this case this lead to a parse error because
> a .rept expression ended up spread over multiple lines. Change the PMDS() 
> invocation to a single line.
> 
> Signed-off-by: Andi Kleen <ak@suse.de>
> 
> Index: linux/arch/x86_64/kernel/head.S
> ===================================================================
> --- linux.orig/arch/x86_64/kernel/head.S
> +++ linux/arch/x86_64/kernel/head.S
> @@ -345,8 +345,7 @@ NEXT_PAGE(level2_kernel_pgt)
>  	/* 40MB kernel mapping. The kernel code cannot be bigger than that.
>  	   When you change this change KERNEL_TEXT_SIZE in page.h too. */
>  	/* (2^48-(2*1024*1024*1024)-((2^39)*511)-((2^30)*510)) = 0 */
> -	PMDS(0x0000000000000000, __PAGE_KERNEL_LARGE_EXEC|_PAGE_GLOBAL,
> -		KERNEL_TEXT_SIZE/PMD_SIZE)
> +	PMDS(0x0000000000000000, __PAGE_KERNEL_LARGE_EXEC|_PAGE_GLOBAL, KERNEL_TEXT_SIZE/PMD_SIZE)

Can you please add a comment, that this line must stay in a single line
for the above reason? I would expect that the next who does some code
clean up will break it again.

>  	/* Module mapping starts here */
>  	.fill	(PTRS_PER_PMD - (KERNEL_TEXT_SIZE/PMD_SIZE)),8,0

Regards,
-- 
Clemens Koller
_______________________________
R&D Imaging Devices
Anagramm GmbH
Rupert-Mayer-Str. 45/1
81379 Muenchen
Germany

http://www.anagramm-technology.com
Phone: +49-89-741518-50
Fax: +49-89-741518-19

Re: [PATCH] [3/4] x86_64: Change PMDS invocation to single macro

[Andi Kleen]

> Can you please add a comment, that this line must stay in a single line
> for the above reason? I would expect that the next who does some code
> clean up will break it again.

Then we would need to add it to all macros essentially. That wouldn't
be economical.

-Andi

[PATCH] [4/4] x86_64: Check for .cfi_rel_offset in CFI probe

[Andi Kleen]

Very old 64bit binutils have .cfi_startproc/endproc, but
no .cfi_rel_offset. Check for .cfi_rel_offset too.

Cc: jbeulich@novell.com
Cc: sam@ravnborg.org

Index: linux/arch/x86_64/Makefile
===================================================================
--- linux.orig/arch/x86_64/Makefile
+++ linux/arch/x86_64/Makefile
@@ -57,8 +57,8 @@ cflags-y += $(call cc-option,-mno-sse -m
 cflags-y += -maccumulate-outgoing-args
 
 # do binutils support CFI?
-cflags-y += $(call as-instr,.cfi_startproc\n.cfi_endproc,-DCONFIG_AS_CFI=1,)
-AFLAGS += $(call as-instr,.cfi_startproc\n.cfi_endproc,-DCONFIG_AS_CFI=1,)
+cflags-y += $(call as-instr,.cfi_startproc\n.cfi_rel_offset rsp${comma}0\n.cfi_endproc,-DCONFIG_AS_CFI=1,)
+AFLAGS += $(call as-instr,.cfi_startproc\n.cfi_rel_offset rsp${comma}0\n.cfi_endproc,-DCONFIG_AS_CFI=1,)
 
 # is .cfi_signal_frame supported too?
 cflags-y += $(call as-instr,.cfi_startproc\n.cfi_signal_frame\n.cfi_endproc,-DCONFIG_AS_CFI_SIGNAL_FRAME=1,)
Index: linux/arch/i386/Makefile
===================================================================
--- linux.orig/arch/i386/Makefile
+++ linux/arch/i386/Makefile
@@ -51,8 +51,8 @@ cflags-y += -maccumulate-outgoing-args
 CFLAGS				+= $(shell if [ $(call cc-version) -lt 0400 ] ; then echo $(call cc-option,-fno-unit-at-a-time); fi ;)
 
 # do binutils support CFI?
-cflags-y += $(call as-instr,.cfi_startproc\n.cfi_endproc,-DCONFIG_AS_CFI=1,)
-AFLAGS += $(call as-instr,.cfi_startproc\n.cfi_endproc,-DCONFIG_AS_CFI=1,)
+cflags-y += $(call as-instr,.cfi_startproc\n.cfi_rel_offset esp${comma}0\n.cfi_endproc,-DCONFIG_AS_CFI=1,)
+AFLAGS += $(call as-instr,.cfi_startproc\n.cfi_rel_offset esp${comma}0\n.cfi_endproc,-DCONFIG_AS_CFI=1,)
 
 # is .cfi_signal_frame supported too?
 cflags-y += $(call as-instr,.cfi_startproc\n.cfi_signal_frame\n.cfi_endproc,-DCONFIG_AS_CFI_SIGNAL_FRAME=1,)