lkml.org 
[lkml]   [2008]   [Jan]   [17]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
    Patch in this message
    /
    Date
    From
    SubjectRe: [patch 0/4] x86: PAT followup - Incremental changes and bug fixes

    * Siddha, Suresh B <suresh.b.siddha@intel.com> 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 <Yinghai.Lu@Sun.COM>
    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 <mingo@elte.hu>
    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 <yinghai.lu@sun.com>

    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;
    } else
    return 1;

    \
     
     \ /
      Last update: 2008-01-17 22:53    [W:0.047 / U:2.616 seconds]
    ©2003-2016 Jasper Spaans. hosted at Digital OceanAdvertise on this site