Re: [patch 0/4] x86: PAT followup - Incremental changes and bug fixes
Eric W. Biederman wrote: | WB WT WC UC ---+--- WB | WB WT WC UC WT | WT WT UC UC WC | WC UC WC UC UC | UC UC UC UC With the current PAT encoding: WB = 00 WT = 01 WC = 10 UC = 11 ... this is simply a bitwise OR. This makes sense, since one of the bits denies delaying writes (WT, UC), and the other denies delaying reads (WC, UC). Almost. There is a specific case and important where MTRR UC + page table WC == WC. But yes. For ioremap where we are WB + MTRR == MTRR we need to request the same attributes as the e820 map, to get the attribute checking correct. True; however, that shouldn't be followed for the case of conflicting attempts at mapping. Now, I *believe* it is safe to have some mappings UC and some WC. This is also something to keep in mind (there are legitimate applications for that particular form of aliasing, too.) If so, we may not want to thump at those. -hpa -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [patch 0/4] x86: PAT followup - Incremental changes and bug fixes
"H. Peter Anvin" <[EMAIL PROTECTED]> writes: > Siddha, Suresh B wrote: >> >> But then, this will cause an attribute conflicit. Old one was specifying >> WB in PAT (ioremap with noflags) and the new ioremap specifies UC. >> >> As Linus mentioned, main problem is to figure out the correct attribute >> for ioremap() which doesn't specify the actual attribute to be used. >> >> One mechanism to fix the issue generically (somewhat atleast) is to use >> MTRR's and figure out the default MTRR attribute for that physical address >> and use it for ioremap(). >> > > This is the matrix the CPU uses when combining MTRR and PAT behaviour. It > probably makes sense to mimic: > >| WB WT WC UC > ---+--- > WB | WB WT WC UC > WT | WT WT UC UC > WC | WC UC WC UC > UC | UC UC UC UC > > With the current PAT encoding: > > WB = 00 > WT = 01 > WC = 10 > UC = 11 > > ... this is simply a bitwise OR. This makes sense, since one of the bits > denies > delaying writes (WT, UC), and the other denies delaying reads (WC, UC). Almost. There is a specific case and important where MTRR UC + page table WC == WC. But yes. For ioremap where we are WB + MTRR == MTRR we need to request the same attributes as the e820 map, to get the attribute checking correct. Eric -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [patch 0/4] x86: PAT followup - Incremental changes and bug fixes
* Pallipadi, Venkatesh <[EMAIL PROTECTED]> [2008-01-18 09:13:10]: > > > >-Original Message- > >From: Andreas Herrmann3 [mailto:[EMAIL PROTECTED] > >Sent: Friday, January 18, 2008 8:11 AM > >To: Pallipadi, Venkatesh > >Cc: Ingo Molnar; Siddha, Suresh B; [EMAIL PROTECTED]; > >[EMAIL PROTECTED]; [EMAIL PROTECTED]; > >[EMAIL PROTECTED]; [EMAIL PROTECTED]; > >[EMAIL PROTECTED]; [EMAIL PROTECTED]; [EMAIL PROTECTED]; > >[EMAIL PROTECTED]; [EMAIL PROTECTED]; [EMAIL PROTECTED]; > >Barnes, Jesse; [EMAIL PROTECTED]; linux-kernel@vger.kernel.org > >Subject: Re: [patch 0/4] x86: PAT followup - Incremental > >changes and bug fixes > > > >On Thu, Jan 17, 2008 at 03:04:10PM -0800, Venki Pallipadi wrote: > >> > >> Below is another potential fix for the problem here. Going > >through ACPI > >> ioremap usages, we found at one place the mapping is cached > >for possible > >> optimization reason and not unmapped later. Patch below always unmaps > >> ioremap at this place in ACPICA. > > > >The patch does not fix the problem. The conflicting cache > >attributes are > >still there. > > > > Andreas, > > Could you also try the patch Suresh Siddha sent out yesterday. That > covers the case where the attribute was not getting removed even after > unmap was called. > An easy way for you to figure out if our patch will solve your problem is this, look for any quirks for your device in drivers/pci/quirks.c and or architecture specific quirks file. If you see your device in there, then our patch is likely to solve your problem. -- Warm Regards, Balbir Singh Linux Technology Center IBM, ISTL -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
RE: [patch 0/4] x86: PAT followup - Incremental changes and bug fixes
>-Original Message- >From: Andreas Herrmann3 [mailto:[EMAIL PROTECTED] >Sent: Friday, January 18, 2008 8:11 AM >To: Pallipadi, Venkatesh >Cc: Ingo Molnar; Siddha, Suresh B; [EMAIL PROTECTED]; >[EMAIL PROTECTED]; [EMAIL PROTECTED]; >[EMAIL PROTECTED]; [EMAIL PROTECTED]; >[EMAIL PROTECTED]; [EMAIL PROTECTED]; [EMAIL PROTECTED]; >[EMAIL PROTECTED]; [EMAIL PROTECTED]; [EMAIL PROTECTED]; >Barnes, Jesse; [EMAIL PROTECTED]; linux-kernel@vger.kernel.org >Subject: Re: [patch 0/4] x86: PAT followup - Incremental >changes and bug fixes > >On Thu, Jan 17, 2008 at 03:04:10PM -0800, Venki Pallipadi wrote: >> >> Below is another potential fix for the problem here. Going >through ACPI >> ioremap usages, we found at one place the mapping is cached >for possible >> optimization reason and not unmapped later. Patch below always unmaps >> ioremap at this place in ACPICA. > >The patch does not fix the problem. The conflicting cache >attributes are >still there. > Andreas, Could you also try the patch Suresh Siddha sent out yesterday. That covers the case where the attribute was not getting removed even after unmap was called. Thanks, Venki -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [patch 0/4] x86: PAT followup - Incremental changes and bug fixes
On Thu, Jan 17, 2008 at 03:04:10PM -0800, Venki Pallipadi wrote: > > Below is another potential fix for the problem here. Going through ACPI > ioremap usages, we found at one place the mapping is cached for possible > optimization reason and not unmapped later. Patch below always unmaps > ioremap at this place in ACPICA. The patch does not fix the problem. The conflicting cache attributes are still there. I have done another test after I have added a debug message in iounmap when a direct mapping gets removed. (See boot-exregion-2.log) For the address in question this gives: # grep c04 boot-exregion-2.log ioremap: addr c0403104, size fc ioremap: addr c0403184, size 7c ioremap: addr c040310a, size f6 ioremap: addr c040310a, size f6 ioremap: addr c040310a, size f6 ioremap: addr c040310a, size f6 ioremap: addr c040318a, size 76 ioremap: addr c0403104, size fc ioremap: addr c0403104, size fc ioremap: addr c0403184, size 7c ioremap: addr c0403104, size fc ioremap: addr c0403184, size 7c ioremap_nocache: addr c040, size 1000 iounmap: addr c040, size 1000 ioremap_nocache: addr c0401000, size 1000 iounmap: addr c0401000, size 1000 ioremap_nocache: addr c0402000, size 1000 iounmap: addr c0402000, size 1000 ioremap: addr c0403104, size fc ioremap: addr c0403184, size 7c ioremap_nocache: addr c0403000, size 200 swapper:1 conflicting cache attribute c0403000-c0404000 uncached<->default ... Regards, Andreas Booting command-list kernel=/tftpboot/aherrma3/hunter-x86-mm-test-vp-debug root=/dev/sda1 console=tt yS0,115200 debug [Linux-bzImage, setup=0x2a00, size=0x24a918] boot Issuing RESET: Linux version 2.6.24-rc8-gd294e9ed-dirty ([EMAIL PROTECTED]) (gcc version 4.1.2 (Gentoo 4.1.2 p1.0.2)) #5 SMP Fri Jan 18 16:47:32 CET 2008 Command line: root=/dev/sda1 console=ttyS0,115200 debug BIOS-provided physical RAM map: BIOS-e820: - 0008dc00 (usable) BIOS-e820: 0008dc00 - 000a (reserved) BIOS-e820: 000d - 0010 (reserved) BIOS-e820: 0010 - 77e7 (usable) BIOS-e820: 77e7 - 77e7a000 (ACPI data) BIOS-e820: 77e7a000 - 77f0 (ACPI NVS) BIOS-e820: 77f0 - 8000 (reserved) BIOS-e820: e000 - f000 (reserved) BIOS-e820: fec0 - fec1 (reserved) BIOS-e820: fee0 - fee01000 (reserved) BIOS-e820: fff0 - 0001 (reserved) Entering add_active_range(0, 0, 141) 0 entries of 3200 used Entering add_active_range(0, 256, 491120) 1 entries of 3200 used end_pfn_map = 1048576 early_ioremap(040e, 0002) => -02105539570 early_iounmap(8280040e, 0002) early_ioremap(0009e000, 0002) => -02104893440 early_iounmap(8289e000, 0002) early_ioremap(8000, 1000) => -02105507840 early_ioremap(9000, 1000) => -02103406592 early_iounmap(82a09000, 1000) early_ioremap(a000, 1000) => -02103402496 early_iounmap(82a0a000, 1000) early_ioremap(b000, 1000) => -02103398400 early_iounmap(82a0b000, 1000) early_ioremap(c000, 1000) => -02103394304 early_iounmap(82a0c000, 1000) early_iounmap(82808000, 1000) early_ioremap(000f, 0001) => -02104557568 DMI present. early_ioremap(77eec000, 0491) => -02102476800 early_iounmap(82aec000, 0491) early_iounmap(828f, 0001) early_ioremap(, 0400) => -02105540608 early_iounmap(8280, 0400) early_ioremap(000e, 0002) => -02104623104 early_iounmap(828e, 0002) early_ioremap(000f7870, 0024) => -02104526736 ACPI: RSDP 000F7870, 0024 (r2 PTLTD ) early_iounmap(828f7870, 0024) early_ioremap(77e72cc4, 0024) => -02105070396 early_iounmap(82872cc4, 0024) early_ioremap(77e72cc4, 004c) => -02105070396 early_iounmap(82872cc4, 004c) early_ioremap(77e72cc4, 0024) => -02105070396 ACPI: XSDT 77E72CC4, 004C (r1 PTLTD XSDT604 LTP0) early_iounmap(82872cc4, 0024) early_ioremap(77e72cc4, 004c) => -02105070396 early_iounmap(82872cc4, 004c) early_ioremap(77e7985c, 0024) => -02105042852 ACPI: FACP 77E7985C, 00F4 (r3 AMDDUNE 604 ATI F4240) early_iounmap(8287985c, 0024) early_ioremap(77e7985c, 00f4) => -02105042852 early_iounmap(8287985c, 00f4) early_ioremap(77e72d10, 0024) => -02105070320 ACPI: DSDT 77E72D10, 6AD8 (r1ATISB400 604 MSFT 10E) early_iounmap(82872d10, 0024) early_ioremap(77e7afc0, 0024) => -02105036864 ACPI: FACS 77E7AFC0, 0040 early_iounmap(828
Re: [patch 0/4] x86: PAT followup - Incremental changes and bug fixes
> As Linus mentioned, main problem is to figure out the correct attribute > for ioremap() which doesn't specify the actual attribute to be used. In this case the correct attribute is the one of the underlying MTRR. And if it conflicts with some other mapping that overrides an MTRR the driver was always broken and it should probably error out and be reevaluated/fixed. -Andi -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
RE: [patch 0/4] x86: PAT followup - Incremental changes and bug fixes
>-Original Message-
>From: Andreas Herrmann3 [mailto:[EMAIL PROTECTED]
>Sent: Thursday, January 17, 2008 3:25 PM
>To: Pallipadi, Venkatesh
>Cc: Ingo Molnar; Siddha, Suresh B; [EMAIL PROTECTED];
>[EMAIL PROTECTED]; [EMAIL PROTECTED];
>[EMAIL PROTECTED]; [EMAIL PROTECTED];
>[EMAIL PROTECTED]; [EMAIL PROTECTED]; [EMAIL PROTECTED];
>[EMAIL PROTECTED]; [EMAIL PROTECTED]; [EMAIL PROTECTED];
>Barnes, Jesse; [EMAIL PROTECTED]; linux-kernel@vger.kernel.org
>Subject: Re: [patch 0/4] x86: PAT followup - Incremental
>changes and bug fixes
>
>On Thu, Jan 17, 2008 at 03:04:10PM -0800, Venki Pallipadi wrote:
>>
>> Below is another potential fix for the problem here. Going
>through ACPI
>> ioremap usages, we found at one place the mapping is cached
>for possible
>> optimization reason and not unmapped later. Patch below always unmaps
>> ioremap at this place in ACPICA.
>>
>> Thanks,
>> Venki
>>
>>
>> Index: linux-2.6.git/drivers/acpi/executer/exregion.c
>> ===
>> --- linux-2.6.git.orig/drivers/acpi/executer/exregion.c
>2008-01-17 03:18:39.0 -0800
>> +++ linux-2.6.git/drivers/acpi/executer/exregion.c
>2008-01-17 07:34:33.0 -0800
>> @@ -48,6 +48,8 @@
>> #define _COMPONENT ACPI_EXECUTER
>> ACPI_MODULE_NAME("exregion")
>>
>> +static int ioremap_cache;
>> +
>>
>/**
>*
>> *
>> * FUNCTION:acpi_ex_system_memory_space_handler
>> @@ -249,6 +251,13 @@
>> break;
>> }
>>
>> +if (!ioremap_cache) {
>> +acpi_os_unmap_memory(mem_info->mapped_logical_address,
>> + window_size);
>> +mem_info->mapped_logical_address = 0;
>> +mem_info->mapped_physical_address = 0;
>> +mem_info->mapped_length = 0;
>> +}
>> return_ACPI_STATUS(status);
>> }
>>
>
>
>Applying and compiling your patch I see:
>
> CC drivers/acpi/executer/exregion.o
>drivers/acpi/executer/exregion.c: In function
>'acpi_ex_system_memory_space_handler':
>drivers/acpi/executer/exregion.c:81: warning: 'window_size'
>may be used uninitialized in this function
>
>
>After glancing through this file it seems that ioremap_cache
>is always 0
>and acpi_os_unmap_memory will unconditionally be executed at
>end of this function.
>I am not familiar with that code. But I just want to reinsure that this
>is what you want. And if so, why is that variable needed?
>But maybe I missed something ...
I missed that warning. But should not matter for testing this patch as
we always initialize window_size with the patch.
Yes. The variable is not needed. With patch I always map at the
beginning of this function and unmap at the end. I just kept the
variable as I was planning to add a boot option to control this
initially. But, later decided to keep the test patch simple without any
boot option.
We can come up with a better patch once we know that the test patch
helps.
Thanks,
Venki
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to [EMAIL PROTECTED]
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
Re: [patch 0/4] x86: PAT followup - Incremental changes and bug fixes
On Thu, Jan 17, 2008 at 03:04:10PM -0800, Venki Pallipadi wrote:
>
> Below is another potential fix for the problem here. Going through ACPI
> ioremap usages, we found at one place the mapping is cached for possible
> optimization reason and not unmapped later. Patch below always unmaps
> ioremap at this place in ACPICA.
>
> Thanks,
> Venki
>
>
> Index: linux-2.6.git/drivers/acpi/executer/exregion.c
> ===
> --- linux-2.6.git.orig/drivers/acpi/executer/exregion.c 2008-01-17
> 03:18:39.0 -0800
> +++ linux-2.6.git/drivers/acpi/executer/exregion.c2008-01-17
> 07:34:33.0 -0800
> @@ -48,6 +48,8 @@
> #define _COMPONENT ACPI_EXECUTER
> ACPI_MODULE_NAME("exregion")
>
> +static int ioremap_cache;
> +
>
> /***
> *
> * FUNCTION:acpi_ex_system_memory_space_handler
> @@ -249,6 +251,13 @@
> break;
> }
>
> + if (!ioremap_cache) {
> + acpi_os_unmap_memory(mem_info->mapped_logical_address,
> + window_size);
> + mem_info->mapped_logical_address = 0;
> + mem_info->mapped_physical_address = 0;
> + mem_info->mapped_length = 0;
> + }
> return_ACPI_STATUS(status);
> }
>
Applying and compiling your patch I see:
CC drivers/acpi/executer/exregion.o
drivers/acpi/executer/exregion.c: In function
'acpi_ex_system_memory_space_handler':
drivers/acpi/executer/exregion.c:81: warning: 'window_size' may be used
uninitialized in this function
After glancing through this file it seems that ioremap_cache is always 0
and acpi_os_unmap_memory will unconditionally be executed at end of this
function.
I am not familiar with that code. But I just want to reinsure that this
is what you want. And if so, why is that variable needed?
But maybe I missed something ...
(I'll test it tomorrow, or I better should say later today.)
Andreas
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to [EMAIL PROTECTED]
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
Re: [patch 0/4] x86: PAT followup - Incremental changes and bug fixes
On Thu, Jan 17, 2008 at 11:35:51PM +0100, Ingo Molnar wrote: > > * Andreas Herrmann3 <[EMAIL PROTECTED]> wrote: > > > > I'll check this asap > > > > So, now that I've avoided this tiny NULL-pointer-dereference, the > > system boots fine as well with your (slightly modified) patch. See > > dmesg attached. > > for now i applied your ioremap_uncached() patch and removed my patch. > > my patch might work if the MTRR marks that area UC. Does it on your > system? The region (c0403000-c04031ff) is not characterized by an MTRR. The MTRRdefType specifies it and it is MTRRdefType = 0x0c00 MemType=0 MtrrDefTypeFixEn=0x1 MtrrDefTypeEn=0x1 => 0==UC So, that's why it worked. Andreas -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [patch 0/4] x86: PAT followup - Incremental changes and bug fixes
On Thu, Jan 17, 2008 at 11:52:43PM +0100, Andreas Herrmann3 wrote:
> On Thu, Jan 17, 2008 at 11:15:05PM +0100, Ingo Molnar wrote:
> >
> > * Andreas Herrmann3 <[EMAIL PROTECTED]> wrote:
> >
> > > On Thu, Jan 17, 2008 at 10:42:09PM +0100, Ingo Molnar wrote:
> > > >
> > > > * Siddha, Suresh B <[EMAIL PROTECTED]> wrote:
> > > >
> > > > > On Thu, Jan 17, 2008 at 10:13:08PM +0100, Ingo Molnar wrote:
> > > > > > but in general we must be robust enough in this case and just
> > > > > > degrade
> > > > > > any overlapping page to UC (and emit a warning perhaps) - instead
> > > > > > of
> > > > > > failing the ioremap and thus failing the driver (and the bootup).
> > > > >
> > > > > But then, this will cause an attribute conflicit. Old one was
> > > > > specifying WB in PAT (ioremap with noflags) and the new ioremap
> > > > > specifies UC.
> > > >
> > > > we could fix up all aliases of that page as well and degrade them to UC?
> > >
> > > Yes, we must fix all aliases or reject the conflicting mapping. But
> > > fixing all aliases might not be that easy. (I've just seen a panic
> > > when using your patch ;-(
> >
> > yes, indeed my patch is bad if you have PAT enabled: conflicting cache
> > attributes might be present. I'll go with your patch for now.
>
> I think the best is to just reject conflicting mappings. (Because now
> I am too tired to think about a safe way how to change the aliases to the
> most restrictive memory type. ;-)
>
> But then of course such boot-time problems like I've seen on my test
> machines should be avoided somehow.
>
>
Below is another potential fix for the problem here. Going through ACPI
ioremap usages, we found at one place the mapping is cached for possible
optimization reason and not unmapped later. Patch below always unmaps
ioremap at this place in ACPICA.
Thanks,
Venki
Index: linux-2.6.git/drivers/acpi/executer/exregion.c
===
--- linux-2.6.git.orig/drivers/acpi/executer/exregion.c 2008-01-17
03:18:39.0 -0800
+++ linux-2.6.git/drivers/acpi/executer/exregion.c 2008-01-17
07:34:33.0 -0800
@@ -48,6 +48,8 @@
#define _COMPONENT ACPI_EXECUTER
ACPI_MODULE_NAME("exregion")
+static int ioremap_cache;
+
/***
*
* FUNCTION:acpi_ex_system_memory_space_handler
@@ -249,6 +251,13 @@
break;
}
+ if (!ioremap_cache) {
+ acpi_os_unmap_memory(mem_info->mapped_logical_address,
+window_size);
+ mem_info->mapped_logical_address = 0;
+ mem_info->mapped_physical_address = 0;
+ mem_info->mapped_length = 0;
+ }
return_ACPI_STATUS(status);
}
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to [EMAIL PROTECTED]
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
Re: [patch 0/4] x86: PAT followup - Incremental changes and bug fixes
On Thu, Jan 17, 2008 at 11:15:05PM +0100, Ingo Molnar wrote: > > * Andreas Herrmann3 <[EMAIL PROTECTED]> wrote: > > > On Thu, Jan 17, 2008 at 10:42:09PM +0100, Ingo Molnar wrote: > > > > > > * Siddha, Suresh B <[EMAIL PROTECTED]> wrote: > > > > > > > On Thu, Jan 17, 2008 at 10:13:08PM +0100, Ingo Molnar wrote: > > > > > but in general we must be robust enough in this case and just degrade > > > > > any overlapping page to UC (and emit a warning perhaps) - instead of > > > > > failing the ioremap and thus failing the driver (and the bootup). > > > > > > > > But then, this will cause an attribute conflicit. Old one was > > > > specifying WB in PAT (ioremap with noflags) and the new ioremap > > > > specifies UC. > > > > > > we could fix up all aliases of that page as well and degrade them to UC? > > > > Yes, we must fix all aliases or reject the conflicting mapping. But > > fixing all aliases might not be that easy. (I've just seen a panic > > when using your patch ;-( > > yes, indeed my patch is bad if you have PAT enabled: conflicting cache > attributes might be present. I'll go with your patch for now. I think the best is to just reject conflicting mappings. (Because now I am too tired to think about a safe way how to change the aliases to the most restrictive memory type. ;-) But then of course such boot-time problems like I've seen on my test machines should be avoided somehow. Andreas -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [patch 0/4] x86: PAT followup - Incremental changes and bug fixes
* Andreas Herrmann3 <[EMAIL PROTECTED]> wrote: > > I'll check this asap > > So, now that I've avoided this tiny NULL-pointer-dereference, the > system boots fine as well with your (slightly modified) patch. See > dmesg attached. for now i applied your ioremap_uncached() patch and removed my patch. my patch might work if the MTRR marks that area UC. Does it on your system? if the MTRRs (as set up by the BIOS) keep it at WB, then the ACPI ioremap() is already unsafe: the mmio area that happens to be there might be prefetched by the CPU. Ingo -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [patch 0/4] x86: PAT followup - Incremental changes and bug fixes
On Thu, Jan 17, 2008 at 10:42:28PM +0100, Andreas Herrmann3 wrote: > On Thu, Jan 17, 2008 at 10:13:08PM +0100, Ingo Molnar wrote: > > > > * Andreas Herrmann3 <[EMAIL PROTECTED]> wrote: > > > > > Yes. > > > > > > Meanwhile I have figured out that it is some ACPI stuff that maps the > > > page cached. I've changed the ioremap's in drivers/acpi/osl.c to > > > ioremap_nocache. See attached patch. > > > > > > Now the machine boots without conflicts. > > > > ah, nice! > > > > but in general we must be robust enough in this case and just degrade > > any overlapping page to UC (and emit a warning perhaps) - instead of > > failing the ioremap and thus failing the driver (and the bootup). > > > > Does my third patch (which falls back to UC in case of attribute > > conflicts, also attached below) instead of your ioremap_nocache() patch > > solve your bootup problem too? > > I'll check this asap So, now that I've avoided this tiny NULL-pointer-dereference, the system boots fine as well with your (slightly modified) patch. See dmesg attached. Andreas Linux version 2.6.24-rc8-gd294e9ed-dirty ([EMAIL PROTECTED]) (gcc version 4.1.2 (Gentoo 4.1.2 p1.0.2)) #2 SMP Thu Jan 17 23:13:37 CET 2008 Command line: root=/dev/sda1 console=ttyS0,115200 debug BIOS-provided physical RAM map: BIOS-e820: - 0008dc00 (usable) BIOS-e820: 0008dc00 - 000a (reserved) BIOS-e820: 000d - 0010 (reserved) BIOS-e820: 0010 - 77e7 (usable) BIOS-e820: 77e7 - 77e7a000 (ACPI data) BIOS-e820: 77e7a000 - 77f0 (ACPI NVS) BIOS-e820: 77f0 - 8000 (reserved) BIOS-e820: e000 - f000 (reserved) BIOS-e820: fec0 - fec1 (reserved) BIOS-e820: fee0 - fee01000 (reserved) BIOS-e820: fff0 - 0001 (reserved) Entering add_active_range(0, 0, 141) 0 entries of 3200 used Entering add_active_range(0, 256, 491120) 1 entries of 3200 used end_pfn_map = 1048576 early_ioremap(040e, 0002) => -02105539570 early_iounmap(8280040e, 0002) early_ioremap(0009e000, 0002) => -02104893440 early_iounmap(8289e000, 0002) early_ioremap(8000, 1000) => -02105507840 early_ioremap(9000, 1000) => -02103406592 early_iounmap(82a09000, 1000) early_ioremap(a000, 1000) => -02103402496 early_iounmap(82a0a000, 1000) early_ioremap(b000, 1000) => -02103398400 early_iounmap(82a0b000, 1000) early_ioremap(c000, 1000) => -02103394304 early_iounmap(82a0c000, 1000) early_iounmap(82808000, 1000) early_ioremap(000f, 0001) => -02104557568 DMI present. early_ioremap(77eec000, 0491) => -02102476800 early_iounmap(82aec000, 0491) early_iounmap(828f, 0001) early_ioremap(, 0400) => -02105540608 early_iounmap(8280, 0400) early_ioremap(000e, 0002) => -02104623104 early_iounmap(828e, 0002) early_ioremap(000f7870, 0024) => -02104526736 ACPI: RSDP 000F7870, 0024 (r2 PTLTD ) early_iounmap(828f7870, 0024) early_ioremap(77e72cc4, 0024) => -02105070396 early_iounmap(82872cc4, 0024) early_ioremap(77e72cc4, 004c) => -02105070396 early_iounmap(82872cc4, 004c) early_ioremap(77e72cc4, 0024) => -02105070396 ACPI: XSDT 77E72CC4, 004C (r1 PTLTD XSDT604 LTP0) early_iounmap(82872cc4, 0024) early_ioremap(77e72cc4, 004c) => -02105070396 early_iounmap(82872cc4, 004c) early_ioremap(77e7985c, 0024) => -02105042852 ACPI: FACP 77E7985C, 00F4 (r3 AMDDUNE 604 ATI F4240) early_iounmap(8287985c, 0024) early_ioremap(77e7985c, 00f4) => -02105042852 early_iounmap(8287985c, 00f4) early_ioremap(77e72d10, 0024) => -02105070320 ACPI: DSDT 77E72D10, 6AD8 (r1ATISB400 604 MSFT 10E) early_iounmap(82872d10, 0024) early_ioremap(77e7afc0, 0024) => -02105036864 ACPI: FACS 77E7AFC0, 0040 early_iounmap(8287afc0, 0024) early_ioremap(77e79950, 0024) => -02105042608 ACPI: TCPA 77E79950, 0032 (r1 Phoeni x604 TL 0) early_iounmap(82879950, 0024) early_ioremap(77e79982, 0024) => -02105042558 ACPI: SSDT 77E79982, 0544 (r1 AMDPOWERNOW 604 AMD 1) early_iounmap(82879982, 0024) early_ioremap(77e79ec6, 0024) => -02105041210 ACPI: SRAT 77E79EC6, 00C0 (r1 AMDHAMMER604 AMD 1) early_iounmap(82879ec6, 0024) early_ioremap(0
Re: [patch 0/4] x86: PAT followup - Incremental changes and bug fixes
On Thu, Jan 17, 2008 at 10:42:28PM +0100, Andreas Herrmann3 wrote: > On Thu, Jan 17, 2008 at 10:13:08PM +0100, Ingo Molnar wrote: > > > > * Andreas Herrmann3 <[EMAIL PROTECTED]> wrote: > > > > > Yes. > > > > > > Meanwhile I have figured out that it is some ACPI stuff that maps the > > > page cached. I've changed the ioremap's in drivers/acpi/osl.c to > > > ioremap_nocache. See attached patch. > > > > > > Now the machine boots without conflicts. > > > > ah, nice! > > > > but in general we must be robust enough in this case and just degrade > > any overlapping page to UC (and emit a warning perhaps) - instead of > > failing the ioremap and thus failing the driver (and the bootup). > > > > Does my third patch (which falls back to UC in case of attribute > > conflicts, also attached below) instead of your ioremap_nocache() patch > > solve your bootup problem too? > > I'll check this asap Ok, here is the result: sata_sil :00:12.0: version 2.3 ACPI: PCI Interrupt :00:12.0[A] -> GSI 22 (level, low) -> IRQ 22 ioremap_nocache: addr c0403000, size 200 swapper:1 conflicting cache attribute c0403000-c0404000 uncached<->default Unable to handle kernel NULL pointer dereference at RIP: [] ? reserve_mat 1a5/0x221 PGD 0 Oops: [1] SMP CPU 3 Modules linked in: Pid: 1, comm: swapper Not tainted 2.6.24-rc8-gd294e9ed-dirty #1 RIP: 0010:[] [] ? reserve_mattr+0x1a5/0x221 RSP: 0018:810077581c60 EFLAGS: 00010282 RAX: 004e RBX: 8100775a7a00 RCX: 4c12 RDX: a9a9 RSI: 0018 RDI: 8153bed4 RBP: R08: 81540fe7 R09: 81329d70 R10: R11: R12: c0404000 R13: 0018 R14: c0403000 R15: c0403000 FS: () GS:8100775d6bc0() knlGS: CS: 0010 DS: 0018 ES: 0018 CR0: 8005003b CR2: CR3: 01001000 CR4: 06e0 DR0: DR1: DR2: DR3: DR6: 0ff0 DR7: 0400 Process swapper (pid: 1, threadinfo 81007758, task 810077564790) Stack: 81411900 1000 1000 c0404000 c28ac000 c0403000 8100775a7a40 810281e9 0018 0005 810077631680 8100777b7800 Call Trace: [] __ioremap+0xc2/0x11a [] pcim_iomap+0x43/0x53 [] pcim_iomap_regions+0x8f/0x104 [] sil_init_one+0xb0/0x1eb [] pci_device_probe+0xd1/0x138 [] driver_probe_device+0xe1/0x16a [] __driver_attach+0x90/0xcd [] __driver_attach+0x0/0xcd [] __driver_attach+0x0/0xcd [] bus_for_each_dev+0x43/0x6e [] bus_add_driver+0x77/0x1be [] __pci_register_driver+0x58/0x8a [] kernel_init+0x170/0x2e0 [] child_rip+0xa/0x12 [] kernel_init+0x0/0x2e0 [] child_rip+0x0/0x12 Code: 00 49 89 c9 48 81 c6 e0 02 00 00 48 89 3c 24 31 c0 4d 89 e0 4c 89 f1 48 c7 c7 c3 97 3e 81 e8 71 ef 00 00 48 c7 43 10 18 00 00 00 <48> 83 3c 25 00 00 00 00 00 74 36 48 c7 04 25 00 00 00 00 18 00 RIP [] ? reserve_mattr+0x1a5/0x221 RSP CR2: ---[ end trace 5516cbea98bb72f9 ]--- Kernel panic - not syncing: Attempted to kill init! I should have reviewed your patch. I guess it must be "if (fattr)" instead of "if (*fattr)" I'll give it another try ... Andreas -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [patch 0/4] x86: PAT followup - Incremental changes and bug fixes
* Andreas Herrmann3 <[EMAIL PROTECTED]> wrote: > On Thu, Jan 17, 2008 at 10:42:09PM +0100, Ingo Molnar wrote: > > > > * Siddha, Suresh B <[EMAIL PROTECTED]> wrote: > > > > > On Thu, Jan 17, 2008 at 10:13:08PM +0100, Ingo Molnar wrote: > > > > but in general we must be robust enough in this case and just degrade > > > > any overlapping page to UC (and emit a warning perhaps) - instead of > > > > failing the ioremap and thus failing the driver (and the bootup). > > > > > > But then, this will cause an attribute conflicit. Old one was > > > specifying WB in PAT (ioremap with noflags) and the new ioremap > > > specifies UC. > > > > we could fix up all aliases of that page as well and degrade them to UC? > > Yes, we must fix all aliases or reject the conflicting mapping. But > fixing all aliases might not be that easy. (I've just seen a panic > when using your patch ;-( yes, indeed my patch is bad if you have PAT enabled: conflicting cache attributes might be present. I'll go with your patch for now. should we perhaps do UC by default for early_ioremap() as well? Normally those mappings are only temporary - but in case of a leak they might hang around in the pagetables and the CPU might stumble upon them. Also, should early_iounmap() do a wbinvd() [/clflush()] call as well, to be safe? Ingo -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [patch 0/4] x86: PAT followup - Incremental changes and bug fixes
Andreas Herrmann3 wrote: Yes, we must fix all aliases or reject the conflicting mapping. But fixing all aliases might not be that easy. (I've just seen a panic when using your patch ;-( Avoiding inconsistent aliases is definitely fundamental to supporting PAT. -hpa -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [patch 0/4] x86: PAT followup - Incremental changes and bug fixes
* Andreas Herrmann3 <[EMAIL PROTECTED]> wrote: > > but i have not seen this message in your boot log. Could you boot > > with early_ioremap_debug and send us the dmesg - i'm curious which > > ACPI tables are actively mapped while those devices are initialized. > > Hmm, early_ioremap_debug exists only in ioremap_32.c Have to adapt the > 64-bit version first. > > But wait the 64-bit code contains already debug output for this. See > the boot-logs that I have attached to my previous mails. > (Interestingly the code for 64-bit early_io(re/un)map resides not in > ioremap_64.c but in init_64.c.) yeah, it's not unified yet. Ingo -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [patch 0/4] x86: PAT followup - Incremental changes and bug fixes
On Thu, Jan 17, 2008 at 10:42:09PM +0100, Ingo Molnar wrote: > > * Siddha, Suresh B <[EMAIL PROTECTED]> wrote: > > > On Thu, Jan 17, 2008 at 10:13:08PM +0100, Ingo Molnar wrote: > > > but in general we must be robust enough in this case and just degrade > > > any overlapping page to UC (and emit a warning perhaps) - instead of > > > failing the ioremap and thus failing the driver (and the bootup). > > > > But then, this will cause an attribute conflicit. Old one was > > specifying WB in PAT (ioremap with noflags) and the new ioremap > > specifies UC. > > we could fix up all aliases of that page as well and degrade them to UC? Yes, we must fix all aliases or reject the conflicting mapping. But fixing all aliases might not be that easy. (I've just seen a panic when using your patch ;-( Andreas -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [patch 0/4] x86: PAT followup - Incremental changes and bug fixes
Siddha, Suresh B wrote: But then, this will cause an attribute conflicit. Old one was specifying WB in PAT (ioremap with noflags) and the new ioremap specifies UC. As Linus mentioned, main problem is to figure out the correct attribute for ioremap() which doesn't specify the actual attribute to be used. One mechanism to fix the issue generically (somewhat atleast) is to use MTRR's and figure out the default MTRR attribute for that physical address and use it for ioremap(). This is the matrix the CPU uses when combining MTRR and PAT behaviour. It probably makes sense to mimic: | WB WT WC UC ---+--- WB | WB WT WC UC WT | WT WT UC UC WC | WC UC WC UC UC | UC UC UC UC With the current PAT encoding: WB = 00 WT = 01 WC = 10 UC = 11 ... this is simply a bitwise OR. This makes sense, since one of the bits denies delaying writes (WT, UC), and the other denies delaying reads (WC, UC). -hpa -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [patch 0/4] x86: PAT followup - Incremental changes and bug fixes
On Thu, Jan 17, 2008 at 10:13:08PM +0100, Ingo Molnar wrote: > > * Andreas Herrmann3 <[EMAIL PROTECTED]> wrote: > > > Yes. > > > > Meanwhile I have figured out that it is some ACPI stuff that maps the > > page cached. I've changed the ioremap's in drivers/acpi/osl.c to > > ioremap_nocache. See attached patch. > > > > Now the machine boots without conflicts. > > ah, nice! > > but in general we must be robust enough in this case and just degrade > any overlapping page to UC (and emit a warning perhaps) - instead of > failing the ioremap and thus failing the driver (and the bootup). > > Does my third patch (which falls back to UC in case of attribute > conflicts, also attached below) instead of your ioremap_nocache() patch > solve your bootup problem too? I'll check this asap > but i have not seen this message in your boot log. Could you boot with > early_ioremap_debug and send us the dmesg - i'm curious which ACPI > tables are actively mapped while those devices are initialized. Hmm, early_ioremap_debug exists only in ioremap_32.c Have to adapt the 64-bit version first. But wait the 64-bit code contains already debug output for this. See the boot-logs that I have attached to my previous mails. (Interestingly the code for 64-bit early_io(re/un)map resides not in ioremap_64.c but in init_64.c.) Andreas -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [patch 0/4] x86: PAT followup - Incremental changes and bug fixes
* Siddha, Suresh B <[EMAIL PROTECTED]> wrote:
> On Thu, Jan 17, 2008 at 10:13:08PM +0100, Ingo Molnar wrote:
> > but in general we must be robust enough in this case and just degrade
> > any overlapping page to UC (and emit a warning perhaps) - instead of
> > failing the ioremap and thus failing the driver (and the bootup).
>
> But then, this will cause an attribute conflicit. Old one was
> specifying WB in PAT (ioremap with noflags) and the new ioremap
> specifies UC.
we could fix up all aliases of that page as well and degrade them to UC?
> As Linus mentioned, main problem is to figure out the correct
> attribute for ioremap() which doesn't specify the actual attribute to
> be used.
i think the problem is the proximity of some ACPI tables to actual
device mmio areas - they share the same physical page. The ACPI tables
will be mapped WB, the device mmio areas will be UC most of the time.
> One mechanism to fix the issue generically (somewhat atleast) is to
> use MTRR's and figure out the default MTRR attribute for that physical
> address and use it for ioremap().
how would this solve the problem at hand? I dont think it's possible to
guarantee that all the BIOS data pages and mmio areas will have
compatible attributes. BIOS data pages might be in plain RAM that we
intend to map WB. Or they might be in reserved areas near the mmio
addresses.
but if we fixed up aliases (only for that single conflicting page), so
that all mappings are degraded to UC, we'd have uniform behavior all
across and the least amount of surprise to drivers. Hm?
> > but i have not seen this message in your boot log. Could you boot
> > with early_ioremap_debug and send us the dmesg - i'm curious which
> > ACPI tables are actively mapped while those devices are initialized.
>
> In this scenario, ACPI is using ioremap() leaving some dangling
> references. Venki is looking to fix this code. Getting the attribute
> for MTRR for ioremap noflags, might solve some of these issues aswell.
> Will look into this.
ok. Resolving that would be nice anyway because the ACPI table might be
in plain RAM which might be reused by the kernel later on, etc. FYI,
there's also the patch from Yinghai Lu on lkml, for one such dangling
reference problem in the SRAT table.
Ingo
>
From: Yinghai Lu <[EMAIL PROTECTED]>
Subject: [PATCH] x86: copy srat table and unmap in acpi_parse_table
[PATCH] x86: copy srat table and unmap in acpi_parse_table
the old acpi_numa_slit_init was saving old address in early stage acpi_slit
and acpi_parse_table can not unmap address that.
the patch copy the slit in the callback,
so we could unmap table in acpi_parse_table instead of outside track it.
need to revert
"
commit d8d28f25f33c6a035cdfb1d421c79293d16e5c58
Author: Ingo Molnar <[EMAIL PROTECTED]>
Date: Thu Jan 17 15:26:42 2008 +0100
x86: ACPI: fix mapping leaks
ioremap_early() is stateful, hence we cannot tolerate mapping leaks.
"
before appling this patch
Signed-off-by: Yinghai Lu <[EMAIL PROTECTED]>
Index: linux-2.6/arch/x86/mm/srat_64.c
===
--- linux-2.6.orig/arch/x86/mm/srat_64.c
+++ linux-2.6/arch/x86/mm/srat_64.c
@@ -23,7 +23,9 @@
int acpi_numa __initdata;
-static struct acpi_table_slit *acpi_slit;
+static int slit_copied;
+static u64 slit_locality_count;
+static u8 slit_entry[MAX_NUMNODES * MAX_NUMNODES];
static nodemask_t nodes_parsed __initdata;
static struct bootnode nodes[MAX_NUMNODES] __initdata;
@@ -130,7 +132,16 @@ void __init acpi_numa_slit_init(struct a
printk(KERN_INFO "ACPI: SLIT table looks invalid. Not used.\n");
return;
}
- acpi_slit = slit;
+
+ if (slit->locality_count > MAX_NUMNODES)
+ return;
+
+ slit_locality_count = slit->locality_count;
+
+ memcpy(slit_entry, slit->entry,
+ slit_locality_count * slit_locality_count);
+
+ slit_copied = 1;
}
/* Callback for Proximity Domain -> LAPIC mapping */
@@ -502,11 +513,11 @@ int __node_distance(int a, int b)
{
int index;
- if (!acpi_slit)
+ if (!slit_copied)
return null_slit_node_compare(a, b) ? LOCAL_DISTANCE :
REMOTE_DISTANCE;
- index = acpi_slit->locality_count * node_to_pxm(a);
- return acpi_slit->entry[index + node_to_pxm(b)];
+ index = slit_locality_count * node_to_pxm(a);
+ return slit_entry[index + node_to_pxm(b)];
}
EXPORT_SYMBOL(__node_distance);
Index: linux-2.6/drivers/acpi/tables.c
===
--- linux-2.6.orig/drivers/acpi/tables.c
+++ linux-2.6/drivers/acpi/tables.c
@@ -260,6 +260,7 @@ int __init acpi_table_parse(char *id, ac
if (table) {
handler(table);
+ acpi_os_unmap_memory(table, table->length);
return 0;
} el
Re: [patch 0/4] x86: PAT followup - Incremental changes and bug fixes
On Thu, Jan 17, 2008 at 10:13:08PM +0100, Ingo Molnar wrote: > but in general we must be robust enough in this case and just degrade > any overlapping page to UC (and emit a warning perhaps) - instead of > failing the ioremap and thus failing the driver (and the bootup). But then, this will cause an attribute conflicit. Old one was specifying WB in PAT (ioremap with noflags) and the new ioremap specifies UC. As Linus mentioned, main problem is to figure out the correct attribute for ioremap() which doesn't specify the actual attribute to be used. One mechanism to fix the issue generically (somewhat atleast) is to use MTRR's and figure out the default MTRR attribute for that physical address and use it for ioremap(). > no, there should be no such need. There can be "mapping leaks", in that > the mapped object is not unmapped. There's detection code in today's > x86.git that should report something like this if it occurs: > > Debug warning: early ioremap leak of 1 areas detected. > please boot with early_ioremap_debug and report the dmesg. > [ cut here ] > WARNING: at arch/x86/mm/ioremap_32.c:346 () > > but i have not seen this message in your boot log. Could you boot with > early_ioremap_debug and send us the dmesg - i'm curious which ACPI > tables are actively mapped while those devices are initialized. In this scenario, ACPI is using ioremap() leaving some dangling references. Venki is looking to fix this code. Getting the attribute for MTRR for ioremap noflags, might solve some of these issues aswell. Will look into this. thanks, suresh -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [patch 0/4] x86: PAT followup - Incremental changes and bug fixes
* Ingo Molnar <[EMAIL PROTECTED]> wrote: > * Andreas Herrmann3 <[EMAIL PROTECTED]> wrote: > > > Yes. > > > > Meanwhile I have figured out that it is some ACPI stuff that maps the > > page cached. I've changed the ioremap's in drivers/acpi/osl.c to > > ioremap_nocache. See attached patch. > > > > Now the machine boots without conflicts. > > ah, nice! > > but in general we must be robust enough in this case and just degrade > any overlapping page to UC (and emit a warning perhaps) - instead of > failing the ioremap and thus failing the driver (and the bootup). btw., there's a change i did in today's x86.git: _all_ the old BIOS data accesses now go through early_ioremap(). This cleaned up the boot code quite significantly, as it's much more apparent now when we access a BIOS data table. (it also solves the problem when BIOS data pages are in reserved areas that we map via UC or dont map at all) the same happens with all ISA ioremaps as well - no more "low 1MB is treated special" exceptions. [ This also solves the 'EFI puts data pages into really high memory we dont have mapped yet' category of problems that BIOS writers are apparently busy creating right now ;-) ] the downside is that old linear-mapped assumptions might now result in an early fault - boot with earlyprintk=vga or earlyprintk=serial,ttyS0,115200. I fixed most such assumptions already and booted an allyesconfig kernel on both 32-bit and 64-bit x86, but a few more remain still. I've enhanced the early fault printout code as well to make it easier to debug such things, so it should be relatively easy to find the rest. Ingo -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [patch 0/4] x86: PAT followup - Incremental changes and bug fixes
* Andreas Herrmann3 <[EMAIL PROTECTED]> wrote:
> Yes.
>
> Meanwhile I have figured out that it is some ACPI stuff that maps the
> page cached. I've changed the ioremap's in drivers/acpi/osl.c to
> ioremap_nocache. See attached patch.
>
> Now the machine boots without conflicts.
ah, nice!
but in general we must be robust enough in this case and just degrade
any overlapping page to UC (and emit a warning perhaps) - instead of
failing the ioremap and thus failing the driver (and the bootup).
Does my third patch (which falls back to UC in case of attribute
conflicts, also attached below) instead of your ioremap_nocache() patch
solve your bootup problem too?
while ACPI should not hurt too much from using UC mappings, we should
still solve this intelligently and only use UC when needed. (Sane system
makers with a sane layout of IO areas and BIOS areas should not be
punished with UC overhead.)
> > as an intermediate fix, how about following the attribute of the
> > already existing mapping, instead of rejecting the ioremap due to
> > the conflict? I.e. something like below?
>
> I guess it is not a good idea to use an existing cachable attribute if
> the IO-region is non-prefetchable. And in this example there are 3
> devices which are potentially affected:
>
> 00:12.0 IDE interface: ATI Technologies Inc 4379 Serial ATA Controller (rev
> 80) (
> ...
> Memory at c0403000 (32-bit, non-prefetchable) [size=512]
> ...
>
> 00:14.0 SMBus: ATI Technologies Inc IXP SB400 SMBus Controller (rev 82)
> ...
> Memory at c0403400 (32-bit, non-prefetchable) [size=1K]
> ...
>
> 00:14.5 Multimedia audio controller: ATI Technologies Inc IXP SB400 AC'97
> Audio Controller (rev 80)
> ...
> Memory at c0403800 (32-bit, non-prefetchable) [size=256]
> ...
>
> BTW, is there a need for osl.c to map all regions as cached?
no, there should be no such need. There can be "mapping leaks", in that
the mapped object is not unmapped. There's detection code in today's
x86.git that should report something like this if it occurs:
Debug warning: early ioremap leak of 1 areas detected.
please boot with early_ioremap_debug and report the dmesg.
[ cut here ]
WARNING: at arch/x86/mm/ioremap_32.c:346 ()
but i have not seen this message in your boot log. Could you boot with
early_ioremap_debug and send us the dmesg - i'm curious which ACPI
tables are actively mapped while those devices are initialized.
Ingo
-->
Subject: x86: patches/pat-conflict-fixup.patch
From: Ingo Molnar <[EMAIL PROTECTED]>
Signed-off-by: Ingo Molnar <[EMAIL PROTECTED]>
---
arch/x86/mm/pat.c |7 ++-
1 file changed, 6 insertions(+), 1 deletion(-)
Index: linux-x86.q/arch/x86/mm/pat.c
===
--- linux-x86.q.orig/arch/x86/mm/pat.c
+++ linux-x86.q/arch/x86/mm/pat.c
@@ -174,7 +174,12 @@ int reserve_mattr(u64 start, u64 end, un
current->comm, current->pid,
start, end,
cattr_name(attr), cattr_name(ml->attr));
- err = -EBUSY;
+ /*
+* Force UC on a conflict:
+*/
+ ma->attr = _PAGE_UC;
+ if (*fattr)
+ *fattr = _PAGE_UC;
break;
}
} else if (ml->start >= end) {
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to [EMAIL PROTECTED]
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
Re: [patch 0/4] x86: PAT followup - Incremental changes and bug fixes
On Thu, Jan 17, 2008 at 09:36:00PM +0100, Ingo Molnar wrote:
>
> * Andreas Herrmann3 <[EMAIL PROTECTED]> wrote:
>
> > For the failed devices I get:
> >
> > sata_sil :00:12.0: version 2.3
> > ACPI: PCI Interrupt :00:12.0[A] -> GSI 22 (level, low) -> IRQ 22
> > ioremap_nocache: addr c0403000, size 200
> > swapper:1 conflicting cache attribute c0403000-c0404000 uncached<->default
> > ACPI: PCI interrupt for device :00:12.0 disabled
>
> hm, is the problem that the two devices share the same physical page,
> and thus get an overlapping area?
Yes.
Meanwhile I have figured out that it is some ACPI stuff that maps the page
cached.
I've changed the ioremap's in drivers/acpi/osl.c to ioremap_nocache.
See attached patch.
Now the machine boots without conflicts.
ACPI: EC: Look up EC in DSDT
ioremap_nocache: addr c0403104, size fc
ioremap_nocache: addr 77e7ade1, size 3
ioremap_nocache: addr 77e7af04, size 1
...
sata_sil :00:12.0: version 2.3
ACPI: PCI Interrupt :00:12.0[A] -> GSI 22 (level, low) -> IRQ 22
ioremap_nocache: addr c0403000, size 200
scsi0 : sata_sil
scsi1 : sata_sil
ata1: SATA max UDMA/100 mmio [EMAIL PROTECTED] tf 0xc0403080 irq 22
ata2: SATA max UDMA/100 mmio [EMAIL PROTECTED] tf 0xc04030c0 irq 22
ata1: SATA link up 1.5 Gbps (SStatus 113 SControl 300)
...
dmesg-output is attached.
> as an intermediate fix, how about following the attribute of the already
> existing mapping, instead of rejecting the ioremap due to the conflict?
> I.e. something like below?
I guess it is not a good idea to use an existing cachable attribute if the
IO-region is non-prefetchable. And in this example there are 3 devices
which are potentially affected:
00:12.0 IDE interface: ATI Technologies Inc 4379 Serial ATA Controller (rev
80) (
...
Memory at c0403000 (32-bit, non-prefetchable) [size=512]
...
00:14.0 SMBus: ATI Technologies Inc IXP SB400 SMBus Controller (rev 82)
...
Memory at c0403400 (32-bit, non-prefetchable) [size=1K]
...
00:14.5 Multimedia audio controller: ATI Technologies Inc IXP SB400 AC'97
Audio Controller (rev 80)
...
Memory at c0403800 (32-bit, non-prefetchable) [size=256]
...
BTW, is there a need for osl.c to map all regions as cached?
Regards,
Andreas
---
diff --git a/drivers/acpi/osl.c b/drivers/acpi/osl.c
index 1f1ec4a..175e6a4 100644
--- a/drivers/acpi/osl.c
+++ b/drivers/acpi/osl.c
@@ -221,7 +221,7 @@ void __iomem *acpi_os_map_memory(acpi_physical_address
phys, acpi_size size)
/*
* ioremap checks to ensure this is in reserved space
*/
- return ioremap((unsigned long)phys, size);
+ return ioremap_nocache((unsigned long)phys, size);
else
return __acpi_map_table((unsigned long)phys, size);
}
@@ -437,7 +437,7 @@ acpi_os_read_memory(acpi_physical_address phys_addr, u32 *
value, u32 width)
u32 dummy;
void __iomem *virt_addr;
- virt_addr = ioremap(phys_addr, width);
+ virt_addr = ioremap_nocache(phys_addr, width);
if (!value)
value = &dummy;
@@ -465,7 +465,7 @@ acpi_os_write_memory(acpi_physical_address phys_addr, u32
value, u32 width)
{
void __iomem *virt_addr;
- virt_addr = ioremap(phys_addr, width);
+ virt_addr = ioremap_nocache(phys_addr, width);
switch (width) {
case 8:
Linux version 2.6.24-rc8-gd294e9ed-dirty ([EMAIL PROTECTED]) (gcc version 4.1.2
(Gentoo 4.1.2 p1.0.2)) #3 SMP Thu Jan 17 21:43:08 CET 2008
Command line: root=/dev/sda1 console=ttyS0,115200 debug
BIOS-provided physical RAM map:
BIOS-e820: - 0008dc00 (usable)
BIOS-e820: 0008dc00 - 000a (reserved)
BIOS-e820: 000d - 0010 (reserved)
BIOS-e820: 0010 - 77e7 (usable)
BIOS-e820: 77e7 - 77e7a000 (ACPI data)
BIOS-e820: 77e7a000 - 77f0 (ACPI NVS)
BIOS-e820: 77f0 - 8000 (reserved)
BIOS-e820: e000 - f000 (reserved)
BIOS-e820: fec0 - fec1 (reserved)
BIOS-e820: fee0 - fee01000 (reserved)
BIOS-e820: fff0 - 0001 (reserved)
Entering add_active_range(0, 0, 141) 0 entries of 3200 used
Entering add_active_range(0, 256, 491120) 1 entries of 3200 used
end_pfn_map = 1048576
early_ioremap(040e, 0002) => -02105539570
early_iounmap(8280040e, 0002)
early_ioremap(0009e000, 0002) => -02104893440
early_iounmap(8289e000, 0002)
early_ioremap(8000, 1000) => -02105507840
early_ioremap(9000, 1000) => -02103406592
early_iounmap(82a09000, 1000)
early_ioremap(a000, 1000) => -02103402496
early_iounmap(82a0a000, 1000)
early_ioremap(000
Re: [patch 0/4] x86: PAT followup - Incremental changes and bug fixes
On Thu, 17 Jan 2008, H. Peter Anvin wrote: > > > swapper:1 conflicting cache attribute c0403000-c0404000 > > > uncached<->default > > > ACPI: PCI interrupt for device :00:12.0 disabled > > The correct behaviour probably would be to go with the most restrictive > caching behaviour, i.e. uncached in this case. Well, the sad part is that in this case, uncached is the SAME THING as default. So it's not like there is any actual real conflict, other than in a PAT confusion thing. Linus -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [patch 0/4] x86: PAT followup - Incremental changes and bug fixes
* H. Peter Anvin <[EMAIL PROTECTED]> wrote:
>> as an intermediate fix, how about following the attribute of the
>> already existing mapping, instead of rejecting the ioremap due to the
>> conflict? I.e. something like below?
>
> The correct behaviour probably would be to go with the most
> restrictive caching behaviour, i.e. uncached in this case.
yeah. Or, to be on the safest side, forcing UC in this case. We'll have
a warning message anyway, so it wont go unnoticed - but we wont break
drivers.
Ingo
->
Subject: x86: patches/pat-conflict-fixup.patch
From: Ingo Molnar <[EMAIL PROTECTED]>
Signed-off-by: Ingo Molnar <[EMAIL PROTECTED]>
---
arch/x86/mm/pat.c |7 ++-
1 file changed, 6 insertions(+), 1 deletion(-)
Index: linux-x86.q/arch/x86/mm/pat.c
===
--- linux-x86.q.orig/arch/x86/mm/pat.c
+++ linux-x86.q/arch/x86/mm/pat.c
@@ -174,7 +174,12 @@ int reserve_mattr(u64 start, u64 end, un
current->comm, current->pid,
start, end,
cattr_name(attr), cattr_name(ml->attr));
- err = -EBUSY;
+ /*
+* Force UC on a conflict:
+*/
+ ma->attr = _PAGE_UC;
+ if (*fattr)
+ *fattr = _PAGE_UC;
break;
}
} else if (ml->start >= end) {
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to [EMAIL PROTECTED]
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
Re: [patch 0/4] x86: PAT followup - Incremental changes and bug fixes
* Ingo Molnar <[EMAIL PROTECTED]> wrote:
> I.e. something like below?
or the one below. (it even builds)
Ingo
---
arch/x86/mm/pat.c |7 ++-
1 file changed, 6 insertions(+), 1 deletion(-)
Index: linux-x86.q/arch/x86/mm/pat.c
===
--- linux-x86.q.orig/arch/x86/mm/pat.c
+++ linux-x86.q/arch/x86/mm/pat.c
@@ -174,7 +174,12 @@ int reserve_mattr(u64 start, u64 end, un
current->comm, current->pid,
start, end,
cattr_name(attr), cattr_name(ml->attr));
- err = -EBUSY;
+ /*
+* Force the already existing attribute:
+*/
+ ma->attr = ml->attr;
+ if (*fattr)
+ *fattr = ml->attr;
break;
}
} else if (ml->start >= end) {
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to [EMAIL PROTECTED]
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
Re: [patch 0/4] x86: PAT followup - Incremental changes and bug fixes
Ingo Molnar wrote: * Andreas Herrmann3 <[EMAIL PROTECTED]> wrote: For the failed devices I get: sata_sil :00:12.0: version 2.3 ACPI: PCI Interrupt :00:12.0[A] -> GSI 22 (level, low) -> IRQ 22 ioremap_nocache: addr c0403000, size 200 swapper:1 conflicting cache attribute c0403000-c0404000 uncached<->default ACPI: PCI interrupt for device :00:12.0 disabled hm, is the problem that the two devices share the same physical page, and thus get an overlapping area? as an intermediate fix, how about following the attribute of the already existing mapping, instead of rejecting the ioremap due to the conflict? I.e. something like below? The correct behaviour probably would be to go with the most restrictive caching behaviour, i.e. uncached in this case. -hpa -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [patch 0/4] x86: PAT followup - Incremental changes and bug fixes
* Andreas Herrmann3 <[EMAIL PROTECTED]> wrote:
> For the failed devices I get:
>
> sata_sil :00:12.0: version 2.3
> ACPI: PCI Interrupt :00:12.0[A] -> GSI 22 (level, low) -> IRQ 22
> ioremap_nocache: addr c0403000, size 200
> swapper:1 conflicting cache attribute c0403000-c0404000 uncached<->default
> ACPI: PCI interrupt for device :00:12.0 disabled
hm, is the problem that the two devices share the same physical page,
and thus get an overlapping area?
as an intermediate fix, how about following the attribute of the already
existing mapping, instead of rejecting the ioremap due to the conflict?
I.e. something like below?
Ingo
---
arch/x86/mm/pat.c |7 ++-
1 file changed, 6 insertions(+), 1 deletion(-)
Index: linux-x86.q/arch/x86/mm/pat.c
===
--- linux-x86.q.orig/arch/x86/mm/pat.c
+++ linux-x86.q/arch/x86/mm/pat.c
@@ -174,7 +174,12 @@ int reserve_mattr(u64 start, u64 end, un
current->comm, current->pid,
start, end,
cattr_name(attr), cattr_name(ml->attr));
- err = -EBUSY;
+ /*
+* Force the already existing attribute:
+*/
+ ma->attr = ml->attr;
+ if (*fattr)
+ *fatt = ml->attr;
break;
}
} else if (ml->start >= end) {
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to [EMAIL PROTECTED]
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
Re: [patch 0/4] x86: PAT followup - Incremental changes and bug fixes
On Thu, Jan 17, 2008 at 08:12:11PM +0100, Andreas Herrmann3 wrote: > On Wed, Jan 16, 2008 at 12:33:28PM -0800, Venki Pallipadi wrote: > > I guess the conflict for sata is > ioremap: addr c0403104, size fc > ioremap_nocache: addr c0403000, size 200 > > But where does the conflict for the sound card > (ioremap_nocache: addr c0403800, size 100) > come from? Sorry, forget this dumb question. Its the same page as above. Andreas -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [patch 0/4] x86: PAT followup - Incremental changes and bug fixes
On Wed, Jan 16, 2008 at 12:33:28PM -0800, Venki Pallipadi wrote: > This ioremap failing seems to be the real problem. This can be due to > new tracking of ioremaps introduced by PAT patches. We do not allow > conflicting ioremaps to same region. Probably that is happening > in both Sound and sata initialization which results in driver init failing. > > Can you please try the debug patch below over latest x86/mm and boot kernel > with > debug boot option and send us the dmesg from the failure. That will give us > better info about ioremaps. Attached is the boot.log with x86/mm as of today (v2.6.24-rc8-720-gd294e9e). For the failed devices I get: sata_sil :00:12.0: version 2.3 ACPI: PCI Interrupt :00:12.0[A] -> GSI 22 (level, low) -> IRQ 22 ioremap_nocache: addr c0403000, size 200 swapper:1 conflicting cache attribute c0403000-c0404000 uncached<->default ACPI: PCI interrupt for device :00:12.0 disabled and Advanced Linux Sound Architecture Driver Version 1.0.15 (Tue Nov 20 19:16:42 2007 UTC). ACPI: PCI Interrupt :00:14.5[B] -> GSI 17 (level, low) -> IRQ 17 ioremap_nocache: addr c0403800, size 100 swapper:1 conflicting cache attribute c0403000-c0404000 uncached<->default AC'97 space ioremap problem ACPI: PCI interrupt for device :00:14.5 disabled ATI IXP AC97 controller: probe of :00:14.5 failed with error -5 Grepping for ioremap/iounmap gives: ioremap: addr 77e72d10, size 6ad8 ioremap: addr 77e79982, size 544 ioremap: addr 77e7afc0, size 40 ioremap: addr c0403104, size fc ioremap: addr 77e7ade1, size 3 ioremap: addr 77e7af04, size 1 ioremap: addr 77e7985c, size f4 ioremap: addr 77e79950, size 32 ioremap: addr 77e79ec6, size c0 ioremap: addr 77e79f86, size 7a ioremap: addr 77e7af74, size 48 ioremap_nocache: addr c040, size 1000 ioremap_nocache: addr c0401000, size 1000 ioremap_nocache: addr c0402000, size 1000 ioremap_nocache: addr c010, size 80 ioremap_nocache: addr c0403000, size 200 ioremap_nocache: addr c0402000, size 1000 ioremap_nocache: addr c040, size 1000 ioremap_nocache: addr c0401000, size 1000 ioremap_nocache: addr c0403800, size 100 I guess the conflict for sata is ioremap: addr c0403104, size fc ioremap_nocache: addr c0403000, size 200 But where does the conflict for the sound card (ioremap_nocache: addr c0403800, size 100) come from? And what can I do about conflicting regions? Regards, Andreas Booting '(hunter) x86/mm-test' kernel=/tftpboot/aherrma3/hunter-x86-mm-test root=/dev/sda1 console=ttyS0,11520 0 debug [Linux-bzImage, setup=0x2a00, size=0x24bd98] boot Issuing RESET: Linux version 2.6.24-rc8-gd294e9ed ([EMAIL PROTECTED]) (gcc version 4.1.2 (Gentoo 4.1.2 p1.0.2)) #1 SMP Thu Jan 17 18:56:17 CET 2008 Command line: root=/dev/sda1 console=ttyS0,115200 debug BIOS-provided physical RAM map: BIOS-e820: - 0008dc00 (usable) BIOS-e820: 0008dc00 - 000a (reserved) BIOS-e820: 000d - 0010 (reserved) BIOS-e820: 0010 - 77e7 (usable) BIOS-e820: 77e7 - 77e7a000 (ACPI data) BIOS-e820: 77e7a000 - 77f0 (ACPI NVS) BIOS-e820: 77f0 - 8000 (reserved) BIOS-e820: e000 - f000 (reserved) BIOS-e820: fec0 - fec1 (reserved) BIOS-e820: fee0 - fee01000 (reserved) BIOS-e820: fff0 - 0001 (reserved) Entering add_active_range(0, 0, 141) 0 entries of 3200 used Entering add_active_range(0, 256, 491120) 1 entries of 3200 used end_pfn_map = 1048576 early_ioremap(040e, 0002) => -02105539570 early_iounmap(8280040e, 0002) early_ioremap(0009e000, 0002) => -02104893440 early_iounmap(8289e000, 0002) early_ioremap(8000, 1000) => -02105507840 early_ioremap(9000, 1000) => -02103406592 early_iounmap(82a09000, 1000) early_ioremap(a000, 1000) => -02103402496 early_iounmap(82a0a000, 1000) early_ioremap(b000, 1000) => -02103398400 early_iounmap(82a0b000, 1000) early_ioremap(c000, 1000) => -02103394304 early_iounmap(82a0c000, 1000) early_iounmap(82808000, 1000) early_ioremap(000f, 0001) => -02104557568 DMI present. early_ioremap(77eec000, 0491) => -02102476800 early_iounmap(82aec000, 0491) early_iounmap(828f, 0001) early_ioremap(, 0400) => -02105540608 early_iounmap(8280, 0400) early_ioremap(000e, 0002) => -02104623104 early_iounmap(828e, 0002) early_ioremap(000f7870, 0024) => -02104526736 ACPI: RSDP 000F7870, 0024 (r2 PTLTD ) early_iounmap(828f7870, 0024) early_ioremap(77e72cc4, 0024) => -021050
Re: [patch 0/4] x86: PAT followup - Incremental changes and bug fixes
> Yes. Printks are there. But are with KERN_DEBUG now. We should change > them to WARNING atleast. I'm pretty sure they were without KERN_* originally. Another reason why the checkpatch.pl KERN_* warnings suck -- the original state would have been better and I bet you changed it just to shut up the dumb scripts. -Andi -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
RE: [patch 0/4] x86: PAT followup - Incremental changes and bug fixes
>-Original Message- >From: Andi Kleen [mailto:[EMAIL PROTECTED] >Sent: Wednesday, January 16, 2008 2:02 PM >To: Pallipadi, Venkatesh >Cc: Andreas Herrmann; [EMAIL PROTECTED]; >[EMAIL PROTECTED]; [EMAIL PROTECTED]; >[EMAIL PROTECTED]; [EMAIL PROTECTED]; [EMAIL PROTECTED]; >[EMAIL PROTECTED]; [EMAIL PROTECTED]; [EMAIL PROTECTED]; >[EMAIL PROTECTED]; [EMAIL PROTECTED]; Barnes, Jesse; >[EMAIL PROTECTED]; linux-kernel@vger.kernel.org; Siddha, Suresh B >Subject: Re: [patch 0/4] x86: PAT followup - Incremental >changes and bug fixes > >> This ioremap failing seems to be the real problem. This can be due to >> new tracking of ioremaps introduced by PAT patches. We do not allow >> conflicting ioremaps to same region. Probably that is happening > >Normally if there is a conflict there should be a printk (or >at least it was >so in the original mattr code if you haven't changed it) > Yes. Printks are there. But are with KERN_DEBUG now. We should change them to WARNING atleast. Thanks, Venki -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [patch 0/4] x86: PAT followup - Incremental changes and bug fixes
> This ioremap failing seems to be the real problem. This can be due to > new tracking of ioremaps introduced by PAT patches. We do not allow > conflicting ioremaps to same region. Probably that is happening Normally if there is a conflict there should be a printk (or at least it was so in the original mattr code if you haven't changed it) -Andi -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [patch 0/4] x86: PAT followup - Incremental changes and bug fixes
On Wed, Jan 16, 2008 at 07:57:48PM +0100, Andreas Herrmann wrote:
> Hi,
>
> I just want to report that the PAT support in x86/mm causes crashes
> on two of my test machines. On both boxes the SATA detection does
> not work when the PAT support is patched into the kernel.
>
> Symptoms are as follows -- best described by a diff between the
> two boot.logs:
>
> # diff boot-failing.log boot-working.log
>
> -Linux version 2.6.24-rc8-ga9f7faa5 ([EMAIL PROTECTED]) (gcc version ...
> +Linux version 2.6.24-rc8-g2ea3cf43 ([EMAIL PROTECTED]) (gcc version ...
> ...
> early_iounmap(82a0b000, 1000)
> -early_ioremap(c000, 1000) => -02103394304
> -early_iounmap(82a0c000, 1000)
This does not look to be the problem here. We just mapped some new low
address due to possibly a different code path. But, seems to have worked fine.
> early_iounmap(82808000, 1000)
> ...
> -ACPI: PCI interrupt for device :00:12.0 disabled
> -sata_sil: probe of :00:12.0 failed with error -12
> +scsi0 : sata_sil
> +scsi1 : sata_sil
> +ata1: SATA max UDMA/100 mmio [EMAIL PROTECTED] tf 0xc0403080 irq 22
> ...
> -AC'97 space ioremap problem
> -ACPI: PCI interrupt for device :00:14.5 disabled
> -ATI IXP AC97 controller: probe of :00:14.5 failed with error -5
This ioremap failing seems to be the real problem. This can be due to
new tracking of ioremaps introduced by PAT patches. We do not allow
conflicting ioremaps to same region. Probably that is happening
in both Sound and sata initialization which results in driver init failing.
Can you please try the debug patch below over latest x86/mm and boot kernel with
debug boot option and send us the dmesg from the failure. That will give us
better info about ioremaps.
Thanks,
Venki
Index: linux-2.6.git/arch/x86/mm/ioremap_64.c
===
--- linux-2.6.git.orig/arch/x86/mm/ioremap_64.c 2008-01-16 03:38:32.0
-0800
+++ linux-2.6.git/arch/x86/mm/ioremap_64.c 2008-01-16 05:16:28.0
-0800
@@ -150,6 +150,8 @@
void __iomem *ioremap_nocache (unsigned long phys_addr, unsigned long size)
{
+ printk(KERN_DEBUG "ioremap_nocache: addr %lx, size %lx\n",
+ phys_addr, size);
return __ioremap(phys_addr, size, _PAGE_UC);
}
EXPORT_SYMBOL(ioremap_nocache);
Index: linux-2.6.git/include/asm-x86/io_64.h
===
--- linux-2.6.git.orig/include/asm-x86/io_64.h 2008-01-16 03:38:32.0
-0800
+++ linux-2.6.git/include/asm-x86/io_64.h 2008-01-16 05:16:57.0
-0800
@@ -154,6 +154,8 @@
static inline void __iomem * ioremap (unsigned long offset, unsigned long size)
{
+ printk(KERN_DEBUG "ioremap: addr %lx, size %lx\n",
+ offset, size);
return __ioremap(offset, size, 0);
}
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to [EMAIL PROTECTED]
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
Re: [patch 0/4] x86: PAT followup - Incremental changes and bug fixes
* Andreas Herrmann <[EMAIL PROTECTED]> wrote: > I just want to report that the PAT support in x86/mm causes crashes on > two of my test machines. On both boxes the SATA detection does not > work when the PAT support is patched into the kernel. > > Symptoms are as follows -- best described by a diff between the two > boot.logs: > > # diff boot-failing.log boot-working.log > > -Linux version 2.6.24-rc8-ga9f7faa5 ([EMAIL PROTECTED]) (gcc version ... > +Linux version 2.6.24-rc8-g2ea3cf43 ([EMAIL PROTECTED]) (gcc version ... > ... > early_iounmap(82a0b000, 1000) > -early_ioremap(c000, 1000) => -02103394304 > -early_iounmap(82a0c000, 1000) hm, so the early_ioremap() stuff isnt working well enough ... that's the main effect of the PAT patches at the moment: no kernel code will access the low linear mappings (BIOS tables, ACPI data, etc.) directly, it's all done via early_ioremap(). But it's apparently buggy somewhere ... Ingo -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
RE: [patch 0/4] x86: PAT followup - Incremental changes and bug fixes
Sorry. Never mind about e820 map. Somehow I did not notice the boot.log you had attached earlier. Thanks, Venki >-Original Message- >From: Pallipadi, Venkatesh >Sent: Wednesday, January 16, 2008 11:06 AM >To: 'Andreas Herrmann' >Cc: [EMAIL PROTECTED]; [EMAIL PROTECTED]; [EMAIL PROTECTED]; >[EMAIL PROTECTED]; [EMAIL PROTECTED]; >[EMAIL PROTECTED]; [EMAIL PROTECTED]; [EMAIL PROTECTED]; >[EMAIL PROTECTED]; [EMAIL PROTECTED]; [EMAIL PROTECTED]; >[EMAIL PROTECTED]; Barnes, Jesse; [EMAIL PROTECTED]; >linux-kernel@vger.kernel.org >Subject: RE: [patch 0/4] x86: PAT followup - Incremental >changes and bug fixes > > >Can you attach the e820 map from the top of your dmesg. > >Thanks, >Venki > >>-Original Message- >>From: Andreas Herrmann [mailto:[EMAIL PROTECTED] >>Sent: Wednesday, January 16, 2008 10:58 AM >>To: Pallipadi, Venkatesh >>Cc: [EMAIL PROTECTED]; [EMAIL PROTECTED]; [EMAIL PROTECTED]; >>[EMAIL PROTECTED]; [EMAIL PROTECTED]; >>[EMAIL PROTECTED]; [EMAIL PROTECTED]; [EMAIL PROTECTED]; >>[EMAIL PROTECTED]; [EMAIL PROTECTED]; [EMAIL PROTECTED]; >>[EMAIL PROTECTED]; Barnes, Jesse; [EMAIL PROTECTED]; >>linux-kernel@vger.kernel.org >>Subject: Re: [patch 0/4] x86: PAT followup - Incremental >>changes and bug fixes >> >>Hi, >> >>I just want to report that the PAT support in x86/mm causes crashes >>on two of my test machines. On both boxes the SATA detection does >>not work when the PAT support is patched into the kernel. >> >>Symptoms are as follows -- best described by a diff between the >>two boot.logs: >> >># diff boot-failing.log boot-working.log >> >>-Linux version 2.6.24-rc8-ga9f7faa5 ([EMAIL PROTECTED]) (gcc version ... >>+Linux version 2.6.24-rc8-g2ea3cf43 ([EMAIL PROTECTED]) (gcc version ... >>... >> early_iounmap(82a0b000, 1000) >>-early_ioremap(c000, 1000) => -02103394304 >>-early_iounmap(82a0c000, 1000) >> early_iounmap(82808000, 1000) >>... >>-ACPI: PCI interrupt for device :00:12.0 disabled >>-sata_sil: probe of :00:12.0 failed with error -12 >>+scsi0 : sata_sil >>+scsi1 : sata_sil >>+ata1: SATA max UDMA/100 mmio [EMAIL PROTECTED] tf 0xc0403080 irq 22 >>... >>-AC'97 space ioremap problem >>-ACPI: PCI interrupt for device :00:14.5 disabled >>-ATI IXP AC97 controller: probe of :00:14.5 failed with error -5 >> ALSA device list: >>- No soundcards found. >>+ #0: ATI IXP rev 80 with ALC655 at 0xc0403800, irq 17 >>... >>-VFS: Cannot open root device "sda1" or unknown-block(0,0) >>-Please append a correct "root=" boot option; here are the >>available partitions: >>-16004194302 hdc driver: ide-cdrom >>-Kernel panic - not syncing: VFS: Unable to mount root fs on >>unknown-block(0,0) >>+kjournald starting. Commit interval 5 seconds >>+EXT3-fs: mounted filesystem with ordered data mode. >>+VFS: Mounted root (ext3 filesystem) readonly. >>... >> >> >> >>The second test machine uses ahci. But the symptoms are similar. >> >>I performed a git-bisect on x86/mm. Last commit that worked for me was >> >>2ea3cf43fddecbfd66353caafdf73ec21ea3760b (x86: fix >>early_ioremap() ISA window) >> >>The subsequent commits for PAT support introduced the problem. >>I noticed that PAT should be disabled by default, but >>obviously the patches >>still have some side-effect. (Maybe ioremap changes lead to >>the problem?) >> >>Boot-logs are attached: >> >> boot-failing.log for x86/mm as of v2.6.24-rc8-672-ga9f7faa >> boot-working.log for x86/mm as of v2.6.24-rc8-621-g2ea3cf4 >> >>Hopefully it helps to track down the problem. >>Maybe someone has an idea why the PAT patches are causing that >>ominous "PCI interrupt for device ... disabled" messages. >> >> >>Thanks and regards, >> >>Andreas >> -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
RE: [patch 0/4] x86: PAT followup - Incremental changes and bug fixes
Can you attach the e820 map from the top of your dmesg. Thanks, Venki >-Original Message- >From: Andreas Herrmann [mailto:[EMAIL PROTECTED] >Sent: Wednesday, January 16, 2008 10:58 AM >To: Pallipadi, Venkatesh >Cc: [EMAIL PROTECTED]; [EMAIL PROTECTED]; [EMAIL PROTECTED]; >[EMAIL PROTECTED]; [EMAIL PROTECTED]; >[EMAIL PROTECTED]; [EMAIL PROTECTED]; [EMAIL PROTECTED]; >[EMAIL PROTECTED]; [EMAIL PROTECTED]; [EMAIL PROTECTED]; >[EMAIL PROTECTED]; Barnes, Jesse; [EMAIL PROTECTED]; >linux-kernel@vger.kernel.org >Subject: Re: [patch 0/4] x86: PAT followup - Incremental >changes and bug fixes > >Hi, > >I just want to report that the PAT support in x86/mm causes crashes >on two of my test machines. On both boxes the SATA detection does >not work when the PAT support is patched into the kernel. > >Symptoms are as follows -- best described by a diff between the >two boot.logs: > ># diff boot-failing.log boot-working.log > >-Linux version 2.6.24-rc8-ga9f7faa5 ([EMAIL PROTECTED]) (gcc version ... >+Linux version 2.6.24-rc8-g2ea3cf43 ([EMAIL PROTECTED]) (gcc version ... >... > early_iounmap(82a0b000, 1000) >-early_ioremap(c000, 1000) => -02103394304 >-early_iounmap(82a0c000, 1000) > early_iounmap(82808000, 1000) >... >-ACPI: PCI interrupt for device :00:12.0 disabled >-sata_sil: probe of :00:12.0 failed with error -12 >+scsi0 : sata_sil >+scsi1 : sata_sil >+ata1: SATA max UDMA/100 mmio [EMAIL PROTECTED] tf 0xc0403080 irq 22 >... >-AC'97 space ioremap problem >-ACPI: PCI interrupt for device :00:14.5 disabled >-ATI IXP AC97 controller: probe of :00:14.5 failed with error -5 > ALSA device list: >- No soundcards found. >+ #0: ATI IXP rev 80 with ALC655 at 0xc0403800, irq 17 >... >-VFS: Cannot open root device "sda1" or unknown-block(0,0) >-Please append a correct "root=" boot option; here are the >available partitions: >-16004194302 hdc driver: ide-cdrom >-Kernel panic - not syncing: VFS: Unable to mount root fs on >unknown-block(0,0) >+kjournald starting. Commit interval 5 seconds >+EXT3-fs: mounted filesystem with ordered data mode. >+VFS: Mounted root (ext3 filesystem) readonly. >... > > > >The second test machine uses ahci. But the symptoms are similar. > >I performed a git-bisect on x86/mm. Last commit that worked for me was > >2ea3cf43fddecbfd66353caafdf73ec21ea3760b (x86: fix >early_ioremap() ISA window) > >The subsequent commits for PAT support introduced the problem. >I noticed that PAT should be disabled by default, but >obviously the patches >still have some side-effect. (Maybe ioremap changes lead to >the problem?) > >Boot-logs are attached: > > boot-failing.log for x86/mm as of v2.6.24-rc8-672-ga9f7faa > boot-working.log for x86/mm as of v2.6.24-rc8-621-g2ea3cf4 > >Hopefully it helps to track down the problem. >Maybe someone has an idea why the PAT patches are causing that >ominous "PCI interrupt for device ... disabled" messages. > > >Thanks and regards, > >Andreas > -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [patch 0/4] x86: PAT followup - Incremental changes and bug fixes
* [EMAIL PROTECTED] <[EMAIL PROTECTED]> wrote: > Some incremental changes and bug fixes for PAT patchset. The changes > are from the feedback we received earlier. There are few more pending > changes that will follow soon. thanks, applied them to x86.git. Note that PAT is still hardcoded to disabled in arch/x86/mm/pat.c: int __read_mostly pat_disabled = 1; because one of my testsystems failed during bootup. I'll re-check whether these fixes resolve that, and if it passes then we could enable PAT. Ingo -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
[patch 0/4] x86: PAT followup - Incremental changes and bug fixes
Some incremental changes and bug fixes for PAT patchset. The changes are from the feedback we received earlier. There are few more pending changes that will follow soon. Thanks, Venki -- -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
