From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from mail-ob0-f179.google.com (mail-ob0-f179.google.com [209.85.214.179]) by kanga.kvack.org (Postfix) with ESMTP id 726FD829A3 for ; Thu, 12 Mar 2015 13:19:01 -0400 (EDT) Received: by obbnt9 with SMTP id nt9so15375863obb.9 for ; Thu, 12 Mar 2015 10:19:01 -0700 (PDT) Received: from g9t5009.houston.hp.com (g9t5009.houston.hp.com. [15.240.92.67]) by mx.google.com with ESMTPS id n145si4195766oig.14.2015.03.12.10.18.59 for (version=TLSv1.2 cipher=ECDHE-RSA-AES128-GCM-SHA256 bits=128/128); Thu, 12 Mar 2015 10:19:00 -0700 (PDT) From: Toshi Kani Subject: [PATCH v2 0/4] mtrr, mm, x86: Enhance MTRR checks for huge I/O mapping Date: Thu, 12 Mar 2015 11:18:06 -0600 Message-Id: <1426180690-24234-1-git-send-email-toshi.kani@hp.com> Sender: owner-linux-mm@kvack.org List-ID: To: akpm@linux-foundation.org, hpa@zytor.com, tglx@linutronix.de, mingo@redhat.com Cc: linux-mm@kvack.org, x86@kernel.org, linux-kernel@vger.kernel.org, dave.hansen@intel.com, Elliott@hp.com, pebolle@tiscali.nl This patchset enhances MTRR checks for the kernel huge I/O mapping, which was enabled by the patchset below: https://lkml.org/lkml/2015/3/3/589 The following functional changes are made in patch 4/4. - Allow pud_set_huge() and pmd_set_huge() to create a huge page mapping to a range covered by a single MTRR entry of any memory type. - Log a pr_warn() message when a specified PMD map range spans more than a single MTRR entry. Drivers should make a mapping request aligned to a single MTRR entry when the range is covered by MTRRs. Patch 1/4 addresses other review comments to the mapping funcs for better code read-ability. Patch 2/4 and 3/4 are bug fix and clean up to mtrr_type_lookup(). The patchset is based on the -mm tree. --- v2: - Update change logs and comments per review comments. (Ingo Molnar) - Add patch 3/4 to clean up mtrr_type_lookup(). (Ingo Molnar) --- Toshi Kani (4): 1/4 mm, x86: Document return values of mapping funcs 2/4 mtrr, x86: Fix MTRR lookup to handle inclusive entry 3/4 mtrr, x86: Clean up mtrr_type_lookup() 4/4 mtrr, mm, x86: Enhance MTRR checks for KVA huge page mapping --- arch/x86/Kconfig | 2 +- arch/x86/include/asm/mtrr.h | 5 +- arch/x86/kernel/cpu/mtrr/generic.c | 151 +++++++++++++++++++++---------------- arch/x86/mm/pat.c | 4 +- arch/x86/mm/pgtable.c | 53 +++++++++---- 5 files changed, 133 insertions(+), 82 deletions(-) -- To unsubscribe, send a message with 'unsubscribe linux-mm' in the body to majordomo@kvack.org. For more info on Linux MM, see: http://www.linux-mm.org/ . Don't email: email@kvack.org From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from mail-ob0-f177.google.com (mail-ob0-f177.google.com [209.85.214.177]) by kanga.kvack.org (Postfix) with ESMTP id 1080C829A3 for ; Thu, 12 Mar 2015 13:19:03 -0400 (EDT) Received: by obcuz6 with SMTP id uz6so15398510obc.7 for ; Thu, 12 Mar 2015 10:19:02 -0700 (PDT) Received: from g4t3427.houston.hp.com (g4t3427.houston.hp.com. [15.201.208.55]) by mx.google.com with ESMTPS id cm6si4147236oec.73.2015.03.12.10.19.02 for (version=TLSv1.2 cipher=ECDHE-RSA-AES128-GCM-SHA256 bits=128/128); Thu, 12 Mar 2015 10:19:02 -0700 (PDT) From: Toshi Kani Subject: [PATCH v2 1/4] mm, x86: Document return values of mapping funcs Date: Thu, 12 Mar 2015 11:18:07 -0600 Message-Id: <1426180690-24234-2-git-send-email-toshi.kani@hp.com> In-Reply-To: <1426180690-24234-1-git-send-email-toshi.kani@hp.com> References: <1426180690-24234-1-git-send-email-toshi.kani@hp.com> Sender: owner-linux-mm@kvack.org List-ID: To: akpm@linux-foundation.org, hpa@zytor.com, tglx@linutronix.de, mingo@redhat.com Cc: linux-mm@kvack.org, x86@kernel.org, linux-kernel@vger.kernel.org, dave.hansen@intel.com, Elliott@hp.com, pebolle@tiscali.nl, Toshi Kani Document the return values of KVA mapping functions, pud_set_huge(), pmd_set_huge, pud_clear_huge() and pmd_clear_huge(). Simplify the conditions to select HAVE_ARCH_HUGE_VMAP in the Kconfig, since X86_PAE depends on X86_32. There is no functional change in this patch. Signed-off-by: Toshi Kani --- arch/x86/Kconfig | 2 +- arch/x86/mm/pgtable.c | 36 ++++++++++++++++++++++++++++-------- 2 files changed, 29 insertions(+), 9 deletions(-) diff --git a/arch/x86/Kconfig b/arch/x86/Kconfig index 110f6ae..ba5e78e 100644 --- a/arch/x86/Kconfig +++ b/arch/x86/Kconfig @@ -99,7 +99,7 @@ config X86 select IRQ_FORCED_THREADING select HAVE_BPF_JIT if X86_64 select HAVE_ARCH_TRANSPARENT_HUGEPAGE - select HAVE_ARCH_HUGE_VMAP if X86_64 || (X86_32 && X86_PAE) + select HAVE_ARCH_HUGE_VMAP if X86_64 || X86_PAE select ARCH_HAS_SG_CHAIN select CLKEVT_I8253 select ARCH_HAVE_NMI_SAFE_CMPXCHG diff --git a/arch/x86/mm/pgtable.c b/arch/x86/mm/pgtable.c index 0b97d2c..4891fa1 100644 --- a/arch/x86/mm/pgtable.c +++ b/arch/x86/mm/pgtable.c @@ -563,14 +563,19 @@ void native_set_fixmap(enum fixed_addresses idx, phys_addr_t phys, } #ifdef CONFIG_HAVE_ARCH_HUGE_VMAP +/** + * pud_set_huge - setup kernel PUD mapping + * + * MTRR can override PAT memory types with 4KB granularity. Therefore, + * it does not set up a huge page when the range is covered by a non-WB + * type of MTRR. 0xFF indicates that MTRR are disabled. + * + * Return 1 on success, and 0 when no PUD was set. + */ int pud_set_huge(pud_t *pud, phys_addr_t addr, pgprot_t prot) { u8 mtrr; - /* - * Do not use a huge page when the range is covered by non-WB type - * of MTRRs. - */ mtrr = mtrr_type_lookup(addr, addr + PUD_SIZE); if ((mtrr != MTRR_TYPE_WRBACK) && (mtrr != 0xFF)) return 0; @@ -584,14 +589,19 @@ int pud_set_huge(pud_t *pud, phys_addr_t addr, pgprot_t prot) return 1; } +/** + * pmd_set_huge - setup kernel PMD mapping + * + * MTRR can override PAT memory types with 4KB granularity. Therefore, + * it does not set up a huge page when the range is covered by a non-WB + * type of MTRR. 0xFF indicates that MTRR are disabled. + * + * Return 1 on success, and 0 when no PMD was set. + */ int pmd_set_huge(pmd_t *pmd, phys_addr_t addr, pgprot_t prot) { u8 mtrr; - /* - * Do not use a huge page when the range is covered by non-WB type - * of MTRRs. - */ mtrr = mtrr_type_lookup(addr, addr + PMD_SIZE); if ((mtrr != MTRR_TYPE_WRBACK) && (mtrr != 0xFF)) return 0; @@ -605,6 +615,11 @@ int pmd_set_huge(pmd_t *pmd, phys_addr_t addr, pgprot_t prot) return 1; } +/** + * pud_clear_huge - clear kernel PUD mapping when it is set + * + * Return 1 on success, and 0 when no PUD map was found. + */ int pud_clear_huge(pud_t *pud) { if (pud_large(*pud)) { @@ -615,6 +630,11 @@ int pud_clear_huge(pud_t *pud) return 0; } +/** + * pmd_clear_huge - clear kernel PMD mapping when it is set + * + * Return 1 on success, and 0 when no PMD map was found. + */ int pmd_clear_huge(pmd_t *pmd) { if (pmd_large(*pmd)) { -- To unsubscribe, send a message with 'unsubscribe linux-mm' in the body to majordomo@kvack.org. For more info on Linux MM, see: http://www.linux-mm.org/ . Don't email: email@kvack.org From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from mail-oi0-f52.google.com (mail-oi0-f52.google.com [209.85.218.52]) by kanga.kvack.org (Postfix) with ESMTP id 17691829A3 for ; Thu, 12 Mar 2015 13:19:05 -0400 (EDT) Received: by oiga141 with SMTP id a141so15799583oig.13 for ; Thu, 12 Mar 2015 10:19:04 -0700 (PDT) Received: from g4t3425.houston.hp.com (g4t3425.houston.hp.com. [15.201.208.53]) by mx.google.com with ESMTPS id t82si2159039oib.86.2015.03.12.10.19.04 for (version=TLSv1.2 cipher=ECDHE-RSA-AES128-GCM-SHA256 bits=128/128); Thu, 12 Mar 2015 10:19:04 -0700 (PDT) From: Toshi Kani Subject: [PATCH v2 2/4] mtrr, x86: Fix MTRR lookup to handle inclusive entry Date: Thu, 12 Mar 2015 11:18:08 -0600 Message-Id: <1426180690-24234-3-git-send-email-toshi.kani@hp.com> In-Reply-To: <1426180690-24234-1-git-send-email-toshi.kani@hp.com> References: <1426180690-24234-1-git-send-email-toshi.kani@hp.com> Sender: owner-linux-mm@kvack.org List-ID: To: akpm@linux-foundation.org, hpa@zytor.com, tglx@linutronix.de, mingo@redhat.com Cc: linux-mm@kvack.org, x86@kernel.org, linux-kernel@vger.kernel.org, dave.hansen@intel.com, Elliott@hp.com, pebolle@tiscali.nl, Toshi Kani When an MTRR entry is inclusive to a requested range, i.e. the start and end of the request are not within the MTRR entry range but the range contains the MTRR entry entirely, __mtrr_type_lookup() ignores such a case because both start_state and end_state are set to zero. This patch fixes the issue by adding a new flag, 'inclusive', to detect the case. This case is then handled in the same way as (!start_state && end_state). Signed-off-by: Toshi Kani --- arch/x86/kernel/cpu/mtrr/generic.c | 17 +++++++++-------- 1 file changed, 9 insertions(+), 8 deletions(-) diff --git a/arch/x86/kernel/cpu/mtrr/generic.c b/arch/x86/kernel/cpu/mtrr/generic.c index 7d74f7b..a82e370 100644 --- a/arch/x86/kernel/cpu/mtrr/generic.c +++ b/arch/x86/kernel/cpu/mtrr/generic.c @@ -154,7 +154,7 @@ static u8 __mtrr_type_lookup(u64 start, u64 end, u64 *partial_end, int *repeat) prev_match = 0xFF; for (i = 0; i < num_var_ranges; ++i) { - unsigned short start_state, end_state; + unsigned short start_state, end_state, inclusive; if (!(mtrr_state.var_ranges[i].mask_lo & (1 << 11))) continue; @@ -166,15 +166,16 @@ static u8 __mtrr_type_lookup(u64 start, u64 end, u64 *partial_end, int *repeat) start_state = ((start & mask) == (base & mask)); end_state = ((end & mask) == (base & mask)); + inclusive = ((start < base) && (end > base)); - if (start_state != end_state) { + if ((start_state != end_state) || inclusive) { /* * We have start:end spanning across an MTRR. - * We split the region into - * either - * (start:mtrr_end) (mtrr_end:end) - * or - * (start:mtrr_start) (mtrr_start:end) + * We split the region into either + * - start_state:1 + * (start:mtrr_end) (mtrr_end:end) + * - end_state:1 or inclusive:1 + * (start:mtrr_start) (mtrr_start:end) * depending on kind of overlap. * Return the type for first region and a pointer to * the start of second region so that caller will @@ -195,7 +196,7 @@ static u8 __mtrr_type_lookup(u64 start, u64 end, u64 *partial_end, int *repeat) *repeat = 1; } - if ((start & mask) != (base & mask)) + if (!start_state) continue; curr_match = mtrr_state.var_ranges[i].base_lo & 0xff; -- To unsubscribe, send a message with 'unsubscribe linux-mm' in the body to majordomo@kvack.org. For more info on Linux MM, see: http://www.linux-mm.org/ . Don't email: email@kvack.org From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from mail-ob0-f182.google.com (mail-ob0-f182.google.com [209.85.214.182]) by kanga.kvack.org (Postfix) with ESMTP id 9565E829A3 for ; Thu, 12 Mar 2015 13:19:07 -0400 (EDT) Received: by obcvb8 with SMTP id vb8so15441998obc.0 for ; Thu, 12 Mar 2015 10:19:07 -0700 (PDT) Received: from g4t3427.houston.hp.com (g4t3427.houston.hp.com. [15.201.208.55]) by mx.google.com with ESMTPS id sb9si4187357obb.24.2015.03.12.10.19.06 for (version=TLSv1.2 cipher=ECDHE-RSA-AES128-GCM-SHA256 bits=128/128); Thu, 12 Mar 2015 10:19:06 -0700 (PDT) From: Toshi Kani Subject: [PATCH v2 3/4] mtrr, x86: Clean up mtrr_type_lookup() Date: Thu, 12 Mar 2015 11:18:09 -0600 Message-Id: <1426180690-24234-4-git-send-email-toshi.kani@hp.com> In-Reply-To: <1426180690-24234-1-git-send-email-toshi.kani@hp.com> References: <1426180690-24234-1-git-send-email-toshi.kani@hp.com> Sender: owner-linux-mm@kvack.org List-ID: To: akpm@linux-foundation.org, hpa@zytor.com, tglx@linutronix.de, mingo@redhat.com Cc: linux-mm@kvack.org, x86@kernel.org, linux-kernel@vger.kernel.org, dave.hansen@intel.com, Elliott@hp.com, pebolle@tiscali.nl, Toshi Kani MTRRs contain fixed and variable entries. mtrr_type_lookup() may repeatedly call __mtrr_type_lookup() to handle a request that overlaps with variable entries. However, __mtrr_type_lookup() also handles the fixed entries and other conditions, which do not have to be repeated. This patch moves such code from __mtrr_type_lookup() to mtrr_type_lookup(). This patch also changes the 'else if (start < 0x1000000)', which checks a fixed range but has an extra zero in the address, to 'else' with no condition. Lastly, the patch updates the function headers to clarify the return values and output argument. It also updates comments to clarify that the repeating is necessary to handle overlaps with the default type, since overlaps with multiple entries alone can be handled without such repeating. There is no functional change in this patch. Signed-off-by: Toshi Kani --- arch/x86/kernel/cpu/mtrr/generic.c | 102 +++++++++++++++++++----------------- 1 file changed, 53 insertions(+), 49 deletions(-) diff --git a/arch/x86/kernel/cpu/mtrr/generic.c b/arch/x86/kernel/cpu/mtrr/generic.c index a82e370..ef34a4f 100644 --- a/arch/x86/kernel/cpu/mtrr/generic.c +++ b/arch/x86/kernel/cpu/mtrr/generic.c @@ -102,12 +102,16 @@ static int check_type_overlap(u8 *prev, u8 *curr) return 0; } -/* - * Error/Semi-error returns: - * 0xFF - when MTRR is not enabled - * *repeat == 1 implies [start:end] spanned across MTRR range and type returned - * corresponds only to [start:*partial_end]. - * Caller has to lookup again for [*partial_end:end]. +/** + * __mtrr_type_lookup - look up memory type in MTRR variable entries + * + * Return Value: + * memory type - Matched memory type or the default memory type (unmatched) + * + * Output Argument: + * repeat - Set to 1 when [start:end] spanned across MTRR range and type + * returned corresponds only to [start:*partial_end]. Caller has + * to lookup again for [*partial_end:end]. */ static u8 __mtrr_type_lookup(u64 start, u64 end, u64 *partial_end, int *repeat) { @@ -116,42 +120,10 @@ static u8 __mtrr_type_lookup(u64 start, u64 end, u64 *partial_end, int *repeat) u8 prev_match, curr_match; *repeat = 0; - if (!mtrr_state_set) - return 0xFF; - - if (!mtrr_state.enabled) - return 0xFF; /* Make end inclusive end, instead of exclusive */ end--; - /* Look in fixed ranges. Just return the type as per start */ - if (mtrr_state.have_fixed && (start < 0x100000)) { - int idx; - - if (start < 0x80000) { - idx = 0; - idx += (start >> 16); - return mtrr_state.fixed_ranges[idx]; - } else if (start < 0xC0000) { - idx = 1 * 8; - idx += ((start - 0x80000) >> 14); - return mtrr_state.fixed_ranges[idx]; - } else if (start < 0x1000000) { - idx = 3 * 8; - idx += ((start - 0xC0000) >> 12); - return mtrr_state.fixed_ranges[idx]; - } - } - - /* - * Look in variable ranges - * Look of multiple ranges matching this address and pick type - * as per MTRR precedence - */ - if (!(mtrr_state.enabled & 2)) - return mtrr_state.def_type; - prev_match = 0xFF; for (i = 0; i < num_var_ranges; ++i) { unsigned short start_state, end_state, inclusive; @@ -180,7 +152,8 @@ static u8 __mtrr_type_lookup(u64 start, u64 end, u64 *partial_end, int *repeat) * Return the type for first region and a pointer to * the start of second region so that caller will * lookup again on the second region. - * Note: This way we handle multiple overlaps as well. + * Note: This way we handle overlaps with multiple + * entries and the default type properly. */ if (start_state) *partial_end = base + get_mtrr_size(mask); @@ -209,21 +182,18 @@ static u8 __mtrr_type_lookup(u64 start, u64 end, u64 *partial_end, int *repeat) return curr_match; } - if (mtrr_tom2) { - if (start >= (1ULL<<32) && (end < mtrr_tom2)) - return MTRR_TYPE_WRBACK; - } - if (prev_match != 0xFF) return prev_match; return mtrr_state.def_type; } -/* - * Returns the effective MTRR type for the region - * Error return: - * 0xFF - when MTRR is not enabled +/** + * mtrr_type_lookup - look up memory type in MTRR + * + * Return Values: + * memory type - The effective MTRR type for the region + * 0xFF - MTRR is disabled */ u8 mtrr_type_lookup(u64 start, u64 end) { @@ -231,12 +201,43 @@ u8 mtrr_type_lookup(u64 start, u64 end) int repeat; u64 partial_end; + if (!mtrr_state_set || !mtrr_state.enabled) + return 0xFF; + + /* Look in fixed ranges. Just return the type as per start */ + if (mtrr_state.have_fixed && (start < 0x100000)) { + int idx; + + if (start < 0x80000) { + idx = 0; + idx += (start >> 16); + return mtrr_state.fixed_ranges[idx]; + } else if (start < 0xC0000) { + idx = 1 * 8; + idx += ((start - 0x80000) >> 14); + return mtrr_state.fixed_ranges[idx]; + } else { + idx = 3 * 8; + idx += ((start - 0xC0000) >> 12); + return mtrr_state.fixed_ranges[idx]; + } + } + + /* + * Look in variable ranges + * Look of multiple ranges matching this address and pick type + * as per MTRR precedence + */ + if (!(mtrr_state.enabled & 2)) + return mtrr_state.def_type; + type = __mtrr_type_lookup(start, end, &partial_end, &repeat); /* * Common path is with repeat = 0. * However, we can have cases where [start:end] spans across some - * MTRR range. Do repeated lookups for that case here. + * MTRR ranges and/or the default type. Do repeated lookups for + * that case here. */ while (repeat) { prev_type = type; @@ -247,6 +248,9 @@ u8 mtrr_type_lookup(u64 start, u64 end) return type; } + if (mtrr_tom2 && (start >= (1ULL<<32)) && (end < mtrr_tom2)) + return MTRR_TYPE_WRBACK; + return type; } -- To unsubscribe, send a message with 'unsubscribe linux-mm' in the body to majordomo@kvack.org. For more info on Linux MM, see: http://www.linux-mm.org/ . Don't email: email@kvack.org From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from mail-oi0-f44.google.com (mail-oi0-f44.google.com [209.85.218.44]) by kanga.kvack.org (Postfix) with ESMTP id 3BCD2829A3 for ; Thu, 12 Mar 2015 13:19:09 -0400 (EDT) Received: by oiga141 with SMTP id a141so15822390oig.8 for ; Thu, 12 Mar 2015 10:19:09 -0700 (PDT) Received: from g9t5009.houston.hp.com (g9t5009.houston.hp.com. [15.240.92.67]) by mx.google.com with ESMTPS id e125si918283oid.131.2015.03.12.10.19.07 for (version=TLSv1.2 cipher=ECDHE-RSA-AES128-GCM-SHA256 bits=128/128); Thu, 12 Mar 2015 10:19:08 -0700 (PDT) From: Toshi Kani Subject: [PATCH v2 4/4] mtrr, mm, x86: Enhance MTRR checks for KVA huge page mapping Date: Thu, 12 Mar 2015 11:18:10 -0600 Message-Id: <1426180690-24234-5-git-send-email-toshi.kani@hp.com> In-Reply-To: <1426180690-24234-1-git-send-email-toshi.kani@hp.com> References: <1426180690-24234-1-git-send-email-toshi.kani@hp.com> Sender: owner-linux-mm@kvack.org List-ID: To: akpm@linux-foundation.org, hpa@zytor.com, tglx@linutronix.de, mingo@redhat.com Cc: linux-mm@kvack.org, x86@kernel.org, linux-kernel@vger.kernel.org, dave.hansen@intel.com, Elliott@hp.com, pebolle@tiscali.nl, Toshi Kani This patch adds an additional argument, 'uniform', to mtrr_type_lookup(), which returns 1 when a given range is covered uniformly by MTRRs, i.e. the range is fully convered by a single MTRR entry or the default type. pud_set_huge() and pmd_set_huge() are changed to check the new 'uniform' flag to see if it is safe to create a huge page mapping to the range. This allows them to create a huge page mapping to a range covered by a single MTRR entry of any memory type. It also detects a non-optimal request properly. They continue to check with the WB type since the WB type has no effect even if a request spans multiple MTRR entries. pmd_set_huge() logs a warning message to a non-optimal request so that driver writers will be aware of such a case. Drivers should make a mapping request aligned to a single MTRR entry when the range is covered by MTRRs. Signed-off-by: Toshi Kani --- arch/x86/include/asm/mtrr.h | 5 +++-- arch/x86/kernel/cpu/mtrr/generic.c | 34 +++++++++++++++++++++++++++------- arch/x86/mm/pat.c | 4 ++-- arch/x86/mm/pgtable.c | 25 +++++++++++++++---------- 4 files changed, 47 insertions(+), 21 deletions(-) diff --git a/arch/x86/include/asm/mtrr.h b/arch/x86/include/asm/mtrr.h index f768f62..5b4d467 100644 --- a/arch/x86/include/asm/mtrr.h +++ b/arch/x86/include/asm/mtrr.h @@ -31,7 +31,7 @@ * arch_phys_wc_add and arch_phys_wc_del. */ # ifdef CONFIG_MTRR -extern u8 mtrr_type_lookup(u64 addr, u64 end); +extern u8 mtrr_type_lookup(u64 addr, u64 end, u8 *uniform); extern void mtrr_save_fixed_ranges(void *); extern void mtrr_save_state(void); extern int mtrr_add(unsigned long base, unsigned long size, @@ -50,11 +50,12 @@ extern int mtrr_trim_uncached_memory(unsigned long end_pfn); extern int amd_special_default_mtrr(void); extern int phys_wc_to_mtrr_index(int handle); # else -static inline u8 mtrr_type_lookup(u64 addr, u64 end) +static inline u8 mtrr_type_lookup(u64 addr, u64 end, u8 *uniform) { /* * Return no-MTRRs: */ + *uniform = 1; return 0xff; } #define mtrr_save_fixed_ranges(arg) do {} while (0) diff --git a/arch/x86/kernel/cpu/mtrr/generic.c b/arch/x86/kernel/cpu/mtrr/generic.c index ef34a4f..56c352c 100644 --- a/arch/x86/kernel/cpu/mtrr/generic.c +++ b/arch/x86/kernel/cpu/mtrr/generic.c @@ -108,18 +108,22 @@ static int check_type_overlap(u8 *prev, u8 *curr) * Return Value: * memory type - Matched memory type or the default memory type (unmatched) * - * Output Argument: + * Output Arguments: * repeat - Set to 1 when [start:end] spanned across MTRR range and type * returned corresponds only to [start:*partial_end]. Caller has * to lookup again for [*partial_end:end]. + * uniform - Set to 1 when MTRR covers the region uniformly, i.e. the region + * is fully covered by a single MTRR entry or the default type. */ -static u8 __mtrr_type_lookup(u64 start, u64 end, u64 *partial_end, int *repeat) +static u8 __mtrr_type_lookup(u64 start, u64 end, + u64 *partial_end, int *repeat, u8 *uniform) { int i; u64 base, mask; u8 prev_match, curr_match; *repeat = 0; + *uniform = 1; /* Make end inclusive end, instead of exclusive */ end--; @@ -167,6 +171,7 @@ static u8 __mtrr_type_lookup(u64 start, u64 end, u64 *partial_end, int *repeat) end = *partial_end - 1; /* end is inclusive */ *repeat = 1; + *uniform = 0; } if (!start_state) @@ -178,6 +183,7 @@ static u8 __mtrr_type_lookup(u64 start, u64 end, u64 *partial_end, int *repeat) continue; } + *uniform = 0; if (check_type_overlap(&prev_match, &curr_match)) return curr_match; } @@ -194,19 +200,26 @@ static u8 __mtrr_type_lookup(u64 start, u64 end, u64 *partial_end, int *repeat) * Return Values: * memory type - The effective MTRR type for the region * 0xFF - MTRR is disabled + * + * Output Argument: + * uniform - Set to 1 when MTRR covers the region uniformly, i.e. the region + * is fully covered by a single MTRR entry or the default type. */ -u8 mtrr_type_lookup(u64 start, u64 end) +u8 mtrr_type_lookup(u64 start, u64 end, u8 *uniform) { - u8 type, prev_type; + u8 type, prev_type, is_uniform, dummy; int repeat; u64 partial_end; + *uniform = 1; + if (!mtrr_state_set || !mtrr_state.enabled) return 0xFF; /* Look in fixed ranges. Just return the type as per start */ if (mtrr_state.have_fixed && (start < 0x100000)) { int idx; + *uniform = 0; if (start < 0x80000) { idx = 0; @@ -231,7 +244,8 @@ u8 mtrr_type_lookup(u64 start, u64 end) if (!(mtrr_state.enabled & 2)) return mtrr_state.def_type; - type = __mtrr_type_lookup(start, end, &partial_end, &repeat); + type = __mtrr_type_lookup(start, end, + &partial_end, &repeat, &is_uniform); /* * Common path is with repeat = 0. @@ -242,15 +256,21 @@ u8 mtrr_type_lookup(u64 start, u64 end) while (repeat) { prev_type = type; start = partial_end; - type = __mtrr_type_lookup(start, end, &partial_end, &repeat); + is_uniform = 0; + + type = __mtrr_type_lookup(start, end, + &partial_end, &repeat, &dummy); - if (check_type_overlap(&prev_type, &type)) + if (check_type_overlap(&prev_type, &type)) { + *uniform = 0; return type; + } } if (mtrr_tom2 && (start >= (1ULL<<32)) && (end < mtrr_tom2)) return MTRR_TYPE_WRBACK; + *uniform = is_uniform; return type; } diff --git a/arch/x86/mm/pat.c b/arch/x86/mm/pat.c index 35af677..372ad42 100644 --- a/arch/x86/mm/pat.c +++ b/arch/x86/mm/pat.c @@ -267,9 +267,9 @@ static unsigned long pat_x_mtrr_type(u64 start, u64 end, * request is for WB. */ if (req_type == _PAGE_CACHE_MODE_WB) { - u8 mtrr_type; + u8 mtrr_type, uniform; - mtrr_type = mtrr_type_lookup(start, end); + mtrr_type = mtrr_type_lookup(start, end, &uniform); if (mtrr_type != MTRR_TYPE_WRBACK) return _PAGE_CACHE_MODE_UC_MINUS; diff --git a/arch/x86/mm/pgtable.c b/arch/x86/mm/pgtable.c index 4891fa1..3d6edea 100644 --- a/arch/x86/mm/pgtable.c +++ b/arch/x86/mm/pgtable.c @@ -567,17 +567,18 @@ void native_set_fixmap(enum fixed_addresses idx, phys_addr_t phys, * pud_set_huge - setup kernel PUD mapping * * MTRR can override PAT memory types with 4KB granularity. Therefore, - * it does not set up a huge page when the range is covered by a non-WB - * type of MTRR. 0xFF indicates that MTRR are disabled. + * it only sets up a huge page when the range is mapped uniformly by MTRR + * (i.e. the range is fully covered by a single MTRR entry or the default + * type) or the MTRR memory type is WB. * * Return 1 on success, and 0 when no PUD was set. */ int pud_set_huge(pud_t *pud, phys_addr_t addr, pgprot_t prot) { - u8 mtrr; + u8 mtrr, uniform; - mtrr = mtrr_type_lookup(addr, addr + PUD_SIZE); - if ((mtrr != MTRR_TYPE_WRBACK) && (mtrr != 0xFF)) + mtrr = mtrr_type_lookup(addr, addr + PUD_SIZE, &uniform); + if ((!uniform) && (mtrr != MTRR_TYPE_WRBACK)) return 0; prot = pgprot_4k_2_large(prot); @@ -593,18 +594,22 @@ int pud_set_huge(pud_t *pud, phys_addr_t addr, pgprot_t prot) * pmd_set_huge - setup kernel PMD mapping * * MTRR can override PAT memory types with 4KB granularity. Therefore, - * it does not set up a huge page when the range is covered by a non-WB - * type of MTRR. 0xFF indicates that MTRR are disabled. + * it only sets up a huge page when the range is mapped uniformly by MTRR + * (i.e. the range is fully covered by a single MTRR entry or the default + * type) or the MTRR memory type is WB. * * Return 1 on success, and 0 when no PMD was set. */ int pmd_set_huge(pmd_t *pmd, phys_addr_t addr, pgprot_t prot) { - u8 mtrr; + u8 mtrr, uniform; - mtrr = mtrr_type_lookup(addr, addr + PMD_SIZE); - if ((mtrr != MTRR_TYPE_WRBACK) && (mtrr != 0xFF)) + mtrr = mtrr_type_lookup(addr, addr + PMD_SIZE, &uniform); + if ((!uniform) && (mtrr != MTRR_TYPE_WRBACK)) { + pr_warn("pmd_set_huge: requesting [mem %#010llx-%#010llx], which spans more than a single MTRR entry\n", + addr, addr + PMD_SIZE); return 0; + } prot = pgprot_4k_2_large(prot); -- To unsubscribe, send a message with 'unsubscribe linux-mm' in the body to majordomo@kvack.org. For more info on Linux MM, see: http://www.linux-mm.org/ . Don't email: email@kvack.org From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from mail-we0-f181.google.com (mail-we0-f181.google.com [74.125.82.181]) by kanga.kvack.org (Postfix) with ESMTP id 47B438299B for ; Fri, 13 Mar 2015 08:37:29 -0400 (EDT) Received: by wesw62 with SMTP id w62so23039749wes.0 for ; Fri, 13 Mar 2015 05:37:28 -0700 (PDT) Received: from mail-wi0-x22c.google.com (mail-wi0-x22c.google.com. [2a00:1450:400c:c05::22c]) by mx.google.com with ESMTPS id j10si2844217wia.115.2015.03.13.05.37.27 for (version=TLSv1.2 cipher=ECDHE-RSA-AES128-GCM-SHA256 bits=128/128); Fri, 13 Mar 2015 05:37:27 -0700 (PDT) Received: by wibbs8 with SMTP id bs8so5843247wib.0 for ; Fri, 13 Mar 2015 05:37:27 -0700 (PDT) Date: Fri, 13 Mar 2015 13:37:23 +0100 From: Ingo Molnar Subject: Re: [PATCH v2 3/4] mtrr, x86: Clean up mtrr_type_lookup() Message-ID: <20150313123722.GA4152@gmail.com> References: <1426180690-24234-1-git-send-email-toshi.kani@hp.com> <1426180690-24234-4-git-send-email-toshi.kani@hp.com> MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: <1426180690-24234-4-git-send-email-toshi.kani@hp.com> Sender: owner-linux-mm@kvack.org List-ID: To: Toshi Kani Cc: akpm@linux-foundation.org, hpa@zytor.com, tglx@linutronix.de, mingo@redhat.com, linux-mm@kvack.org, x86@kernel.org, linux-kernel@vger.kernel.org, dave.hansen@intel.com, Elliott@hp.com, pebolle@tiscali.nl * Toshi Kani wrote: > MTRRs contain fixed and variable entries. mtrr_type_lookup() > may repeatedly call __mtrr_type_lookup() to handle a request > that overlaps with variable entries. However, > __mtrr_type_lookup() also handles the fixed entries and other > conditions, which do not have to be repeated. This patch moves > such code from __mtrr_type_lookup() to mtrr_type_lookup(). > > This patch also changes the 'else if (start < 0x1000000)', > which checks a fixed range but has an extra zero in the address, > to 'else' with no condition. > > Lastly, the patch updates the function headers to clarify the > return values and output argument. It also updates comments to > clarify that the repeating is necessary to handle overlaps with > the default type, since overlaps with multiple entries alone > can be handled without such repeating. > > There is no functional change in this patch. > > Signed-off-by: Toshi Kani > --- > arch/x86/kernel/cpu/mtrr/generic.c | 102 +++++++++++++++++++----------------- > 1 file changed, 53 insertions(+), 49 deletions(-) > > diff --git a/arch/x86/kernel/cpu/mtrr/generic.c b/arch/x86/kernel/cpu/mtrr/generic.c > index a82e370..ef34a4f 100644 > --- a/arch/x86/kernel/cpu/mtrr/generic.c > +++ b/arch/x86/kernel/cpu/mtrr/generic.c > @@ -102,12 +102,16 @@ static int check_type_overlap(u8 *prev, u8 *curr) > return 0; > } > > -/* > - * Error/Semi-error returns: > - * 0xFF - when MTRR is not enabled > - * *repeat == 1 implies [start:end] spanned across MTRR range and type returned > - * corresponds only to [start:*partial_end]. > - * Caller has to lookup again for [*partial_end:end]. > +/** > + * __mtrr_type_lookup - look up memory type in MTRR variable entries > + * > + * Return Value: > + * memory type - Matched memory type or the default memory type (unmatched) > + * > + * Output Argument: > + * repeat - Set to 1 when [start:end] spanned across MTRR range and type > + * returned corresponds only to [start:*partial_end]. Caller has > + * to lookup again for [*partial_end:end]. > */ > static u8 __mtrr_type_lookup(u64 start, u64 end, u64 *partial_end, int *repeat) > { > @@ -116,42 +120,10 @@ static u8 __mtrr_type_lookup(u64 start, u64 end, u64 *partial_end, int *repeat) > u8 prev_match, curr_match; > > *repeat = 0; > - if (!mtrr_state_set) > - return 0xFF; > - > - if (!mtrr_state.enabled) > - return 0xFF; > > /* Make end inclusive end, instead of exclusive */ > end--; > > - /* Look in fixed ranges. Just return the type as per start */ > - if (mtrr_state.have_fixed && (start < 0x100000)) { > - int idx; > - > - if (start < 0x80000) { > - idx = 0; > - idx += (start >> 16); > - return mtrr_state.fixed_ranges[idx]; > - } else if (start < 0xC0000) { > - idx = 1 * 8; > - idx += ((start - 0x80000) >> 14); > - return mtrr_state.fixed_ranges[idx]; > - } else if (start < 0x1000000) { > - idx = 3 * 8; > - idx += ((start - 0xC0000) >> 12); > - return mtrr_state.fixed_ranges[idx]; > - } > - } > - > - /* > - * Look in variable ranges > - * Look of multiple ranges matching this address and pick type > - * as per MTRR precedence > - */ > - if (!(mtrr_state.enabled & 2)) > - return mtrr_state.def_type; > - > prev_match = 0xFF; > for (i = 0; i < num_var_ranges; ++i) { > unsigned short start_state, end_state, inclusive; > @@ -180,7 +152,8 @@ static u8 __mtrr_type_lookup(u64 start, u64 end, u64 *partial_end, int *repeat) > * Return the type for first region and a pointer to > * the start of second region so that caller will > * lookup again on the second region. > - * Note: This way we handle multiple overlaps as well. > + * Note: This way we handle overlaps with multiple > + * entries and the default type properly. > */ > if (start_state) > *partial_end = base + get_mtrr_size(mask); > @@ -209,21 +182,18 @@ static u8 __mtrr_type_lookup(u64 start, u64 end, u64 *partial_end, int *repeat) > return curr_match; > } > > - if (mtrr_tom2) { > - if (start >= (1ULL<<32) && (end < mtrr_tom2)) > - return MTRR_TYPE_WRBACK; > - } > - > if (prev_match != 0xFF) > return prev_match; > > return mtrr_state.def_type; > } > > -/* > - * Returns the effective MTRR type for the region > - * Error return: > - * 0xFF - when MTRR is not enabled > +/** > + * mtrr_type_lookup - look up memory type in MTRR > + * > + * Return Values: > + * memory type - The effective MTRR type for the region > + * 0xFF - MTRR is disabled > */ > u8 mtrr_type_lookup(u64 start, u64 end) > { > @@ -231,12 +201,43 @@ u8 mtrr_type_lookup(u64 start, u64 end) > int repeat; > u64 partial_end; > > + if (!mtrr_state_set || !mtrr_state.enabled) > + return 0xFF; > + > + /* Look in fixed ranges. Just return the type as per start */ > + if (mtrr_state.have_fixed && (start < 0x100000)) { > + int idx; > + > + if (start < 0x80000) { > + idx = 0; > + idx += (start >> 16); > + return mtrr_state.fixed_ranges[idx]; > + } else if (start < 0xC0000) { > + idx = 1 * 8; > + idx += ((start - 0x80000) >> 14); > + return mtrr_state.fixed_ranges[idx]; > + } else { > + idx = 3 * 8; > + idx += ((start - 0xC0000) >> 12); > + return mtrr_state.fixed_ranges[idx]; > + } > + } So why not put this into a separate helper function - named mtrr_type_lookup_fixed()? It has little relation to variable ranges. > + > + /* > + * Look in variable ranges > + * Look of multiple ranges matching this address and pick type > + * as per MTRR precedence > + */ > + if (!(mtrr_state.enabled & 2)) > + return mtrr_state.def_type; > + > type = __mtrr_type_lookup(start, end, &partial_end, &repeat); And this then should be named mtrr_type_lookup_variable() or so? Thanks, Ingo -- To unsubscribe, send a message with 'unsubscribe linux-mm' in the body to majordomo@kvack.org. For more info on Linux MM, see: http://www.linux-mm.org/ . Don't email: email@kvack.org From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from mail-ob0-f177.google.com (mail-ob0-f177.google.com [209.85.214.177]) by kanga.kvack.org (Postfix) with ESMTP id 81D69829B9 for ; Fri, 13 Mar 2015 09:54:02 -0400 (EDT) Received: by obcvb8 with SMTP id vb8so19808899obc.0 for ; Fri, 13 Mar 2015 06:54:02 -0700 (PDT) Received: from g9t5009.houston.hp.com (g9t5009.houston.hp.com. [15.240.92.67]) by mx.google.com with ESMTPS id o10si1085403oex.59.2015.03.13.06.54.01 for (version=TLSv1.2 cipher=ECDHE-RSA-AES128-GCM-SHA256 bits=128/128); Fri, 13 Mar 2015 06:54:01 -0700 (PDT) Message-ID: <1426254791.17007.451.camel@misato.fc.hp.com> Subject: Re: [PATCH v2 3/4] mtrr, x86: Clean up mtrr_type_lookup() From: Toshi Kani Date: Fri, 13 Mar 2015 07:53:11 -0600 In-Reply-To: <20150313123722.GA4152@gmail.com> References: <1426180690-24234-1-git-send-email-toshi.kani@hp.com> <1426180690-24234-4-git-send-email-toshi.kani@hp.com> <20150313123722.GA4152@gmail.com> Content-Type: text/plain; charset="UTF-8" Mime-Version: 1.0 Content-Transfer-Encoding: 7bit Sender: owner-linux-mm@kvack.org List-ID: To: Ingo Molnar Cc: "akpm@linux-foundation.org" , "hpa@zytor.com" , "tglx@linutronix.de" , "mingo@redhat.com" , "linux-mm@kvack.org" , "x86@kernel.org" , "linux-kernel@vger.kernel.org" , "dave.hansen@intel.com" , "Elliott, Robert (Server Storage)" , "pebolle@tiscali.nl" On Fri, 2015-03-13 at 12:37 +0000, Ingo Molnar wrote: > * Toshi Kani wrote: : > > + /* Look in fixed ranges. Just return the type as per start */ > > + if (mtrr_state.have_fixed && (start < 0x100000)) { > > + int idx; > > + > > + if (start < 0x80000) { > > + idx = 0; > > + idx += (start >> 16); > > + return mtrr_state.fixed_ranges[idx]; > > + } else if (start < 0xC0000) { > > + idx = 1 * 8; > > + idx += ((start - 0x80000) >> 14); > > + return mtrr_state.fixed_ranges[idx]; > > + } else { > > + idx = 3 * 8; > > + idx += ((start - 0xC0000) >> 12); > > + return mtrr_state.fixed_ranges[idx]; > > + } > > + } > > So why not put this into a separate helper function - named > mtrr_type_lookup_fixed()? It has little relation to variable ranges. Sounds good. I will update as suggested. > > + > > + /* > > + * Look in variable ranges > > + * Look of multiple ranges matching this address and pick type > > + * as per MTRR precedence > > + */ > > + if (!(mtrr_state.enabled & 2)) > > + return mtrr_state.def_type; > > + > > type = __mtrr_type_lookup(start, end, &partial_end, &repeat); > > And this then should be named mtrr_type_lookup_variable() or so? Will do as well. I will send out a new version today since I won't be able to update the patchset next week. Thanks, -Toshi -- To unsubscribe, send a message with 'unsubscribe linux-mm' in the body to majordomo@kvack.org. For more info on Linux MM, see: http://www.linux-mm.org/ . Don't email: email@kvack.org From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1754984AbbCLRTB (ORCPT ); Thu, 12 Mar 2015 13:19:01 -0400 Received: from g9t5009.houston.hp.com ([15.240.92.67]:40592 "EHLO g9t5009.houston.hp.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1754775AbbCLRTA (ORCPT ); Thu, 12 Mar 2015 13:19:00 -0400 From: Toshi Kani To: akpm@linux-foundation.org, hpa@zytor.com, tglx@linutronix.de, mingo@redhat.com Cc: linux-mm@kvack.org, x86@kernel.org, linux-kernel@vger.kernel.org, dave.hansen@intel.com, Elliott@hp.com, pebolle@tiscali.nl Subject: [PATCH v2 0/4] mtrr, mm, x86: Enhance MTRR checks for huge I/O mapping Date: Thu, 12 Mar 2015 11:18:06 -0600 Message-Id: <1426180690-24234-1-git-send-email-toshi.kani@hp.com> X-Mailer: git-send-email 1.9.3 Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org This patchset enhances MTRR checks for the kernel huge I/O mapping, which was enabled by the patchset below: https://lkml.org/lkml/2015/3/3/589 The following functional changes are made in patch 4/4. - Allow pud_set_huge() and pmd_set_huge() to create a huge page mapping to a range covered by a single MTRR entry of any memory type. - Log a pr_warn() message when a specified PMD map range spans more than a single MTRR entry. Drivers should make a mapping request aligned to a single MTRR entry when the range is covered by MTRRs. Patch 1/4 addresses other review comments to the mapping funcs for better code read-ability. Patch 2/4 and 3/4 are bug fix and clean up to mtrr_type_lookup(). The patchset is based on the -mm tree. --- v2: - Update change logs and comments per review comments. (Ingo Molnar) - Add patch 3/4 to clean up mtrr_type_lookup(). (Ingo Molnar) --- Toshi Kani (4): 1/4 mm, x86: Document return values of mapping funcs 2/4 mtrr, x86: Fix MTRR lookup to handle inclusive entry 3/4 mtrr, x86: Clean up mtrr_type_lookup() 4/4 mtrr, mm, x86: Enhance MTRR checks for KVA huge page mapping --- arch/x86/Kconfig | 2 +- arch/x86/include/asm/mtrr.h | 5 +- arch/x86/kernel/cpu/mtrr/generic.c | 151 +++++++++++++++++++++---------------- arch/x86/mm/pat.c | 4 +- arch/x86/mm/pgtable.c | 53 +++++++++---- 5 files changed, 133 insertions(+), 82 deletions(-) From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1755054AbbCLRTH (ORCPT ); Thu, 12 Mar 2015 13:19:07 -0400 Received: from g4t3427.houston.hp.com ([15.201.208.55]:49025 "EHLO g4t3427.houston.hp.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1754775AbbCLRTD (ORCPT ); Thu, 12 Mar 2015 13:19:03 -0400 From: Toshi Kani To: akpm@linux-foundation.org, hpa@zytor.com, tglx@linutronix.de, mingo@redhat.com Cc: linux-mm@kvack.org, x86@kernel.org, linux-kernel@vger.kernel.org, dave.hansen@intel.com, Elliott@hp.com, pebolle@tiscali.nl, Toshi Kani Subject: [PATCH v2 1/4] mm, x86: Document return values of mapping funcs Date: Thu, 12 Mar 2015 11:18:07 -0600 Message-Id: <1426180690-24234-2-git-send-email-toshi.kani@hp.com> X-Mailer: git-send-email 1.9.3 In-Reply-To: <1426180690-24234-1-git-send-email-toshi.kani@hp.com> References: <1426180690-24234-1-git-send-email-toshi.kani@hp.com> Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org Document the return values of KVA mapping functions, pud_set_huge(), pmd_set_huge, pud_clear_huge() and pmd_clear_huge(). Simplify the conditions to select HAVE_ARCH_HUGE_VMAP in the Kconfig, since X86_PAE depends on X86_32. There is no functional change in this patch. Signed-off-by: Toshi Kani --- arch/x86/Kconfig | 2 +- arch/x86/mm/pgtable.c | 36 ++++++++++++++++++++++++++++-------- 2 files changed, 29 insertions(+), 9 deletions(-) diff --git a/arch/x86/Kconfig b/arch/x86/Kconfig index 110f6ae..ba5e78e 100644 --- a/arch/x86/Kconfig +++ b/arch/x86/Kconfig @@ -99,7 +99,7 @@ config X86 select IRQ_FORCED_THREADING select HAVE_BPF_JIT if X86_64 select HAVE_ARCH_TRANSPARENT_HUGEPAGE - select HAVE_ARCH_HUGE_VMAP if X86_64 || (X86_32 && X86_PAE) + select HAVE_ARCH_HUGE_VMAP if X86_64 || X86_PAE select ARCH_HAS_SG_CHAIN select CLKEVT_I8253 select ARCH_HAVE_NMI_SAFE_CMPXCHG diff --git a/arch/x86/mm/pgtable.c b/arch/x86/mm/pgtable.c index 0b97d2c..4891fa1 100644 --- a/arch/x86/mm/pgtable.c +++ b/arch/x86/mm/pgtable.c @@ -563,14 +563,19 @@ void native_set_fixmap(enum fixed_addresses idx, phys_addr_t phys, } #ifdef CONFIG_HAVE_ARCH_HUGE_VMAP +/** + * pud_set_huge - setup kernel PUD mapping + * + * MTRR can override PAT memory types with 4KB granularity. Therefore, + * it does not set up a huge page when the range is covered by a non-WB + * type of MTRR. 0xFF indicates that MTRR are disabled. + * + * Return 1 on success, and 0 when no PUD was set. + */ int pud_set_huge(pud_t *pud, phys_addr_t addr, pgprot_t prot) { u8 mtrr; - /* - * Do not use a huge page when the range is covered by non-WB type - * of MTRRs. - */ mtrr = mtrr_type_lookup(addr, addr + PUD_SIZE); if ((mtrr != MTRR_TYPE_WRBACK) && (mtrr != 0xFF)) return 0; @@ -584,14 +589,19 @@ int pud_set_huge(pud_t *pud, phys_addr_t addr, pgprot_t prot) return 1; } +/** + * pmd_set_huge - setup kernel PMD mapping + * + * MTRR can override PAT memory types with 4KB granularity. Therefore, + * it does not set up a huge page when the range is covered by a non-WB + * type of MTRR. 0xFF indicates that MTRR are disabled. + * + * Return 1 on success, and 0 when no PMD was set. + */ int pmd_set_huge(pmd_t *pmd, phys_addr_t addr, pgprot_t prot) { u8 mtrr; - /* - * Do not use a huge page when the range is covered by non-WB type - * of MTRRs. - */ mtrr = mtrr_type_lookup(addr, addr + PMD_SIZE); if ((mtrr != MTRR_TYPE_WRBACK) && (mtrr != 0xFF)) return 0; @@ -605,6 +615,11 @@ int pmd_set_huge(pmd_t *pmd, phys_addr_t addr, pgprot_t prot) return 1; } +/** + * pud_clear_huge - clear kernel PUD mapping when it is set + * + * Return 1 on success, and 0 when no PUD map was found. + */ int pud_clear_huge(pud_t *pud) { if (pud_large(*pud)) { @@ -615,6 +630,11 @@ int pud_clear_huge(pud_t *pud) return 0; } +/** + * pmd_clear_huge - clear kernel PMD mapping when it is set + * + * Return 1 on success, and 0 when no PMD map was found. + */ int pmd_clear_huge(pmd_t *pmd) { if (pmd_large(*pmd)) { From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1754761AbbCLRTK (ORCPT ); Thu, 12 Mar 2015 13:19:10 -0400 Received: from g4t3425.houston.hp.com ([15.201.208.53]:59236 "EHLO g4t3425.houston.hp.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1755002AbbCLRTF (ORCPT ); Thu, 12 Mar 2015 13:19:05 -0400 From: Toshi Kani To: akpm@linux-foundation.org, hpa@zytor.com, tglx@linutronix.de, mingo@redhat.com Cc: linux-mm@kvack.org, x86@kernel.org, linux-kernel@vger.kernel.org, dave.hansen@intel.com, Elliott@hp.com, pebolle@tiscali.nl, Toshi Kani Subject: [PATCH v2 2/4] mtrr, x86: Fix MTRR lookup to handle inclusive entry Date: Thu, 12 Mar 2015 11:18:08 -0600 Message-Id: <1426180690-24234-3-git-send-email-toshi.kani@hp.com> X-Mailer: git-send-email 1.9.3 In-Reply-To: <1426180690-24234-1-git-send-email-toshi.kani@hp.com> References: <1426180690-24234-1-git-send-email-toshi.kani@hp.com> Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org When an MTRR entry is inclusive to a requested range, i.e. the start and end of the request are not within the MTRR entry range but the range contains the MTRR entry entirely, __mtrr_type_lookup() ignores such a case because both start_state and end_state are set to zero. This patch fixes the issue by adding a new flag, 'inclusive', to detect the case. This case is then handled in the same way as (!start_state && end_state). Signed-off-by: Toshi Kani --- arch/x86/kernel/cpu/mtrr/generic.c | 17 +++++++++-------- 1 file changed, 9 insertions(+), 8 deletions(-) diff --git a/arch/x86/kernel/cpu/mtrr/generic.c b/arch/x86/kernel/cpu/mtrr/generic.c index 7d74f7b..a82e370 100644 --- a/arch/x86/kernel/cpu/mtrr/generic.c +++ b/arch/x86/kernel/cpu/mtrr/generic.c @@ -154,7 +154,7 @@ static u8 __mtrr_type_lookup(u64 start, u64 end, u64 *partial_end, int *repeat) prev_match = 0xFF; for (i = 0; i < num_var_ranges; ++i) { - unsigned short start_state, end_state; + unsigned short start_state, end_state, inclusive; if (!(mtrr_state.var_ranges[i].mask_lo & (1 << 11))) continue; @@ -166,15 +166,16 @@ static u8 __mtrr_type_lookup(u64 start, u64 end, u64 *partial_end, int *repeat) start_state = ((start & mask) == (base & mask)); end_state = ((end & mask) == (base & mask)); + inclusive = ((start < base) && (end > base)); - if (start_state != end_state) { + if ((start_state != end_state) || inclusive) { /* * We have start:end spanning across an MTRR. - * We split the region into - * either - * (start:mtrr_end) (mtrr_end:end) - * or - * (start:mtrr_start) (mtrr_start:end) + * We split the region into either + * - start_state:1 + * (start:mtrr_end) (mtrr_end:end) + * - end_state:1 or inclusive:1 + * (start:mtrr_start) (mtrr_start:end) * depending on kind of overlap. * Return the type for first region and a pointer to * the start of second region so that caller will @@ -195,7 +196,7 @@ static u8 __mtrr_type_lookup(u64 start, u64 end, u64 *partial_end, int *repeat) *repeat = 1; } - if ((start & mask) != (base & mask)) + if (!start_state) continue; curr_match = mtrr_state.var_ranges[i].base_lo & 0xff; From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1755174AbbCLRTx (ORCPT ); Thu, 12 Mar 2015 13:19:53 -0400 Received: from g4t3427.houston.hp.com ([15.201.208.55]:49092 "EHLO g4t3427.houston.hp.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1755009AbbCLRTG (ORCPT ); Thu, 12 Mar 2015 13:19:06 -0400 From: Toshi Kani To: akpm@linux-foundation.org, hpa@zytor.com, tglx@linutronix.de, mingo@redhat.com Cc: linux-mm@kvack.org, x86@kernel.org, linux-kernel@vger.kernel.org, dave.hansen@intel.com, Elliott@hp.com, pebolle@tiscali.nl, Toshi Kani Subject: [PATCH v2 3/4] mtrr, x86: Clean up mtrr_type_lookup() Date: Thu, 12 Mar 2015 11:18:09 -0600 Message-Id: <1426180690-24234-4-git-send-email-toshi.kani@hp.com> X-Mailer: git-send-email 1.9.3 In-Reply-To: <1426180690-24234-1-git-send-email-toshi.kani@hp.com> References: <1426180690-24234-1-git-send-email-toshi.kani@hp.com> Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org MTRRs contain fixed and variable entries. mtrr_type_lookup() may repeatedly call __mtrr_type_lookup() to handle a request that overlaps with variable entries. However, __mtrr_type_lookup() also handles the fixed entries and other conditions, which do not have to be repeated. This patch moves such code from __mtrr_type_lookup() to mtrr_type_lookup(). This patch also changes the 'else if (start < 0x1000000)', which checks a fixed range but has an extra zero in the address, to 'else' with no condition. Lastly, the patch updates the function headers to clarify the return values and output argument. It also updates comments to clarify that the repeating is necessary to handle overlaps with the default type, since overlaps with multiple entries alone can be handled without such repeating. There is no functional change in this patch. Signed-off-by: Toshi Kani --- arch/x86/kernel/cpu/mtrr/generic.c | 102 +++++++++++++++++++----------------- 1 file changed, 53 insertions(+), 49 deletions(-) diff --git a/arch/x86/kernel/cpu/mtrr/generic.c b/arch/x86/kernel/cpu/mtrr/generic.c index a82e370..ef34a4f 100644 --- a/arch/x86/kernel/cpu/mtrr/generic.c +++ b/arch/x86/kernel/cpu/mtrr/generic.c @@ -102,12 +102,16 @@ static int check_type_overlap(u8 *prev, u8 *curr) return 0; } -/* - * Error/Semi-error returns: - * 0xFF - when MTRR is not enabled - * *repeat == 1 implies [start:end] spanned across MTRR range and type returned - * corresponds only to [start:*partial_end]. - * Caller has to lookup again for [*partial_end:end]. +/** + * __mtrr_type_lookup - look up memory type in MTRR variable entries + * + * Return Value: + * memory type - Matched memory type or the default memory type (unmatched) + * + * Output Argument: + * repeat - Set to 1 when [start:end] spanned across MTRR range and type + * returned corresponds only to [start:*partial_end]. Caller has + * to lookup again for [*partial_end:end]. */ static u8 __mtrr_type_lookup(u64 start, u64 end, u64 *partial_end, int *repeat) { @@ -116,42 +120,10 @@ static u8 __mtrr_type_lookup(u64 start, u64 end, u64 *partial_end, int *repeat) u8 prev_match, curr_match; *repeat = 0; - if (!mtrr_state_set) - return 0xFF; - - if (!mtrr_state.enabled) - return 0xFF; /* Make end inclusive end, instead of exclusive */ end--; - /* Look in fixed ranges. Just return the type as per start */ - if (mtrr_state.have_fixed && (start < 0x100000)) { - int idx; - - if (start < 0x80000) { - idx = 0; - idx += (start >> 16); - return mtrr_state.fixed_ranges[idx]; - } else if (start < 0xC0000) { - idx = 1 * 8; - idx += ((start - 0x80000) >> 14); - return mtrr_state.fixed_ranges[idx]; - } else if (start < 0x1000000) { - idx = 3 * 8; - idx += ((start - 0xC0000) >> 12); - return mtrr_state.fixed_ranges[idx]; - } - } - - /* - * Look in variable ranges - * Look of multiple ranges matching this address and pick type - * as per MTRR precedence - */ - if (!(mtrr_state.enabled & 2)) - return mtrr_state.def_type; - prev_match = 0xFF; for (i = 0; i < num_var_ranges; ++i) { unsigned short start_state, end_state, inclusive; @@ -180,7 +152,8 @@ static u8 __mtrr_type_lookup(u64 start, u64 end, u64 *partial_end, int *repeat) * Return the type for first region and a pointer to * the start of second region so that caller will * lookup again on the second region. - * Note: This way we handle multiple overlaps as well. + * Note: This way we handle overlaps with multiple + * entries and the default type properly. */ if (start_state) *partial_end = base + get_mtrr_size(mask); @@ -209,21 +182,18 @@ static u8 __mtrr_type_lookup(u64 start, u64 end, u64 *partial_end, int *repeat) return curr_match; } - if (mtrr_tom2) { - if (start >= (1ULL<<32) && (end < mtrr_tom2)) - return MTRR_TYPE_WRBACK; - } - if (prev_match != 0xFF) return prev_match; return mtrr_state.def_type; } -/* - * Returns the effective MTRR type for the region - * Error return: - * 0xFF - when MTRR is not enabled +/** + * mtrr_type_lookup - look up memory type in MTRR + * + * Return Values: + * memory type - The effective MTRR type for the region + * 0xFF - MTRR is disabled */ u8 mtrr_type_lookup(u64 start, u64 end) { @@ -231,12 +201,43 @@ u8 mtrr_type_lookup(u64 start, u64 end) int repeat; u64 partial_end; + if (!mtrr_state_set || !mtrr_state.enabled) + return 0xFF; + + /* Look in fixed ranges. Just return the type as per start */ + if (mtrr_state.have_fixed && (start < 0x100000)) { + int idx; + + if (start < 0x80000) { + idx = 0; + idx += (start >> 16); + return mtrr_state.fixed_ranges[idx]; + } else if (start < 0xC0000) { + idx = 1 * 8; + idx += ((start - 0x80000) >> 14); + return mtrr_state.fixed_ranges[idx]; + } else { + idx = 3 * 8; + idx += ((start - 0xC0000) >> 12); + return mtrr_state.fixed_ranges[idx]; + } + } + + /* + * Look in variable ranges + * Look of multiple ranges matching this address and pick type + * as per MTRR precedence + */ + if (!(mtrr_state.enabled & 2)) + return mtrr_state.def_type; + type = __mtrr_type_lookup(start, end, &partial_end, &repeat); /* * Common path is with repeat = 0. * However, we can have cases where [start:end] spans across some - * MTRR range. Do repeated lookups for that case here. + * MTRR ranges and/or the default type. Do repeated lookups for + * that case here. */ while (repeat) { prev_type = type; @@ -247,6 +248,9 @@ u8 mtrr_type_lookup(u64 start, u64 end) return type; } + if (mtrr_tom2 && (start >= (1ULL<<32)) && (end < mtrr_tom2)) + return MTRR_TYPE_WRBACK; + return type; } From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1755107AbbCLRTO (ORCPT ); Thu, 12 Mar 2015 13:19:14 -0400 Received: from g9t5009.houston.hp.com ([15.240.92.67]:40722 "EHLO g9t5009.houston.hp.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1754775AbbCLRTI (ORCPT ); Thu, 12 Mar 2015 13:19:08 -0400 From: Toshi Kani To: akpm@linux-foundation.org, hpa@zytor.com, tglx@linutronix.de, mingo@redhat.com Cc: linux-mm@kvack.org, x86@kernel.org, linux-kernel@vger.kernel.org, dave.hansen@intel.com, Elliott@hp.com, pebolle@tiscali.nl, Toshi Kani Subject: [PATCH v2 4/4] mtrr, mm, x86: Enhance MTRR checks for KVA huge page mapping Date: Thu, 12 Mar 2015 11:18:10 -0600 Message-Id: <1426180690-24234-5-git-send-email-toshi.kani@hp.com> X-Mailer: git-send-email 1.9.3 In-Reply-To: <1426180690-24234-1-git-send-email-toshi.kani@hp.com> References: <1426180690-24234-1-git-send-email-toshi.kani@hp.com> Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org This patch adds an additional argument, 'uniform', to mtrr_type_lookup(), which returns 1 when a given range is covered uniformly by MTRRs, i.e. the range is fully convered by a single MTRR entry or the default type. pud_set_huge() and pmd_set_huge() are changed to check the new 'uniform' flag to see if it is safe to create a huge page mapping to the range. This allows them to create a huge page mapping to a range covered by a single MTRR entry of any memory type. It also detects a non-optimal request properly. They continue to check with the WB type since the WB type has no effect even if a request spans multiple MTRR entries. pmd_set_huge() logs a warning message to a non-optimal request so that driver writers will be aware of such a case. Drivers should make a mapping request aligned to a single MTRR entry when the range is covered by MTRRs. Signed-off-by: Toshi Kani --- arch/x86/include/asm/mtrr.h | 5 +++-- arch/x86/kernel/cpu/mtrr/generic.c | 34 +++++++++++++++++++++++++++------- arch/x86/mm/pat.c | 4 ++-- arch/x86/mm/pgtable.c | 25 +++++++++++++++---------- 4 files changed, 47 insertions(+), 21 deletions(-) diff --git a/arch/x86/include/asm/mtrr.h b/arch/x86/include/asm/mtrr.h index f768f62..5b4d467 100644 --- a/arch/x86/include/asm/mtrr.h +++ b/arch/x86/include/asm/mtrr.h @@ -31,7 +31,7 @@ * arch_phys_wc_add and arch_phys_wc_del. */ # ifdef CONFIG_MTRR -extern u8 mtrr_type_lookup(u64 addr, u64 end); +extern u8 mtrr_type_lookup(u64 addr, u64 end, u8 *uniform); extern void mtrr_save_fixed_ranges(void *); extern void mtrr_save_state(void); extern int mtrr_add(unsigned long base, unsigned long size, @@ -50,11 +50,12 @@ extern int mtrr_trim_uncached_memory(unsigned long end_pfn); extern int amd_special_default_mtrr(void); extern int phys_wc_to_mtrr_index(int handle); # else -static inline u8 mtrr_type_lookup(u64 addr, u64 end) +static inline u8 mtrr_type_lookup(u64 addr, u64 end, u8 *uniform) { /* * Return no-MTRRs: */ + *uniform = 1; return 0xff; } #define mtrr_save_fixed_ranges(arg) do {} while (0) diff --git a/arch/x86/kernel/cpu/mtrr/generic.c b/arch/x86/kernel/cpu/mtrr/generic.c index ef34a4f..56c352c 100644 --- a/arch/x86/kernel/cpu/mtrr/generic.c +++ b/arch/x86/kernel/cpu/mtrr/generic.c @@ -108,18 +108,22 @@ static int check_type_overlap(u8 *prev, u8 *curr) * Return Value: * memory type - Matched memory type or the default memory type (unmatched) * - * Output Argument: + * Output Arguments: * repeat - Set to 1 when [start:end] spanned across MTRR range and type * returned corresponds only to [start:*partial_end]. Caller has * to lookup again for [*partial_end:end]. + * uniform - Set to 1 when MTRR covers the region uniformly, i.e. the region + * is fully covered by a single MTRR entry or the default type. */ -static u8 __mtrr_type_lookup(u64 start, u64 end, u64 *partial_end, int *repeat) +static u8 __mtrr_type_lookup(u64 start, u64 end, + u64 *partial_end, int *repeat, u8 *uniform) { int i; u64 base, mask; u8 prev_match, curr_match; *repeat = 0; + *uniform = 1; /* Make end inclusive end, instead of exclusive */ end--; @@ -167,6 +171,7 @@ static u8 __mtrr_type_lookup(u64 start, u64 end, u64 *partial_end, int *repeat) end = *partial_end - 1; /* end is inclusive */ *repeat = 1; + *uniform = 0; } if (!start_state) @@ -178,6 +183,7 @@ static u8 __mtrr_type_lookup(u64 start, u64 end, u64 *partial_end, int *repeat) continue; } + *uniform = 0; if (check_type_overlap(&prev_match, &curr_match)) return curr_match; } @@ -194,19 +200,26 @@ static u8 __mtrr_type_lookup(u64 start, u64 end, u64 *partial_end, int *repeat) * Return Values: * memory type - The effective MTRR type for the region * 0xFF - MTRR is disabled + * + * Output Argument: + * uniform - Set to 1 when MTRR covers the region uniformly, i.e. the region + * is fully covered by a single MTRR entry or the default type. */ -u8 mtrr_type_lookup(u64 start, u64 end) +u8 mtrr_type_lookup(u64 start, u64 end, u8 *uniform) { - u8 type, prev_type; + u8 type, prev_type, is_uniform, dummy; int repeat; u64 partial_end; + *uniform = 1; + if (!mtrr_state_set || !mtrr_state.enabled) return 0xFF; /* Look in fixed ranges. Just return the type as per start */ if (mtrr_state.have_fixed && (start < 0x100000)) { int idx; + *uniform = 0; if (start < 0x80000) { idx = 0; @@ -231,7 +244,8 @@ u8 mtrr_type_lookup(u64 start, u64 end) if (!(mtrr_state.enabled & 2)) return mtrr_state.def_type; - type = __mtrr_type_lookup(start, end, &partial_end, &repeat); + type = __mtrr_type_lookup(start, end, + &partial_end, &repeat, &is_uniform); /* * Common path is with repeat = 0. @@ -242,15 +256,21 @@ u8 mtrr_type_lookup(u64 start, u64 end) while (repeat) { prev_type = type; start = partial_end; - type = __mtrr_type_lookup(start, end, &partial_end, &repeat); + is_uniform = 0; + + type = __mtrr_type_lookup(start, end, + &partial_end, &repeat, &dummy); - if (check_type_overlap(&prev_type, &type)) + if (check_type_overlap(&prev_type, &type)) { + *uniform = 0; return type; + } } if (mtrr_tom2 && (start >= (1ULL<<32)) && (end < mtrr_tom2)) return MTRR_TYPE_WRBACK; + *uniform = is_uniform; return type; } diff --git a/arch/x86/mm/pat.c b/arch/x86/mm/pat.c index 35af677..372ad42 100644 --- a/arch/x86/mm/pat.c +++ b/arch/x86/mm/pat.c @@ -267,9 +267,9 @@ static unsigned long pat_x_mtrr_type(u64 start, u64 end, * request is for WB. */ if (req_type == _PAGE_CACHE_MODE_WB) { - u8 mtrr_type; + u8 mtrr_type, uniform; - mtrr_type = mtrr_type_lookup(start, end); + mtrr_type = mtrr_type_lookup(start, end, &uniform); if (mtrr_type != MTRR_TYPE_WRBACK) return _PAGE_CACHE_MODE_UC_MINUS; diff --git a/arch/x86/mm/pgtable.c b/arch/x86/mm/pgtable.c index 4891fa1..3d6edea 100644 --- a/arch/x86/mm/pgtable.c +++ b/arch/x86/mm/pgtable.c @@ -567,17 +567,18 @@ void native_set_fixmap(enum fixed_addresses idx, phys_addr_t phys, * pud_set_huge - setup kernel PUD mapping * * MTRR can override PAT memory types with 4KB granularity. Therefore, - * it does not set up a huge page when the range is covered by a non-WB - * type of MTRR. 0xFF indicates that MTRR are disabled. + * it only sets up a huge page when the range is mapped uniformly by MTRR + * (i.e. the range is fully covered by a single MTRR entry or the default + * type) or the MTRR memory type is WB. * * Return 1 on success, and 0 when no PUD was set. */ int pud_set_huge(pud_t *pud, phys_addr_t addr, pgprot_t prot) { - u8 mtrr; + u8 mtrr, uniform; - mtrr = mtrr_type_lookup(addr, addr + PUD_SIZE); - if ((mtrr != MTRR_TYPE_WRBACK) && (mtrr != 0xFF)) + mtrr = mtrr_type_lookup(addr, addr + PUD_SIZE, &uniform); + if ((!uniform) && (mtrr != MTRR_TYPE_WRBACK)) return 0; prot = pgprot_4k_2_large(prot); @@ -593,18 +594,22 @@ int pud_set_huge(pud_t *pud, phys_addr_t addr, pgprot_t prot) * pmd_set_huge - setup kernel PMD mapping * * MTRR can override PAT memory types with 4KB granularity. Therefore, - * it does not set up a huge page when the range is covered by a non-WB - * type of MTRR. 0xFF indicates that MTRR are disabled. + * it only sets up a huge page when the range is mapped uniformly by MTRR + * (i.e. the range is fully covered by a single MTRR entry or the default + * type) or the MTRR memory type is WB. * * Return 1 on success, and 0 when no PMD was set. */ int pmd_set_huge(pmd_t *pmd, phys_addr_t addr, pgprot_t prot) { - u8 mtrr; + u8 mtrr, uniform; - mtrr = mtrr_type_lookup(addr, addr + PMD_SIZE); - if ((mtrr != MTRR_TYPE_WRBACK) && (mtrr != 0xFF)) + mtrr = mtrr_type_lookup(addr, addr + PMD_SIZE, &uniform); + if ((!uniform) && (mtrr != MTRR_TYPE_WRBACK)) { + pr_warn("pmd_set_huge: requesting [mem %#010llx-%#010llx], which spans more than a single MTRR entry\n", + addr, addr + PMD_SIZE); return 0; + } prot = pgprot_4k_2_large(prot); From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1755122AbbCMMhb (ORCPT ); Fri, 13 Mar 2015 08:37:31 -0400 Received: from mail-wi0-f180.google.com ([209.85.212.180]:37379 "EHLO mail-wi0-f180.google.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1751295AbbCMMh2 (ORCPT ); Fri, 13 Mar 2015 08:37:28 -0400 Date: Fri, 13 Mar 2015 13:37:23 +0100 From: Ingo Molnar To: Toshi Kani Cc: akpm@linux-foundation.org, hpa@zytor.com, tglx@linutronix.de, mingo@redhat.com, linux-mm@kvack.org, x86@kernel.org, linux-kernel@vger.kernel.org, dave.hansen@intel.com, Elliott@hp.com, pebolle@tiscali.nl Subject: Re: [PATCH v2 3/4] mtrr, x86: Clean up mtrr_type_lookup() Message-ID: <20150313123722.GA4152@gmail.com> References: <1426180690-24234-1-git-send-email-toshi.kani@hp.com> <1426180690-24234-4-git-send-email-toshi.kani@hp.com> MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: <1426180690-24234-4-git-send-email-toshi.kani@hp.com> User-Agent: Mutt/1.5.23 (2014-03-12) Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org * Toshi Kani wrote: > MTRRs contain fixed and variable entries. mtrr_type_lookup() > may repeatedly call __mtrr_type_lookup() to handle a request > that overlaps with variable entries. However, > __mtrr_type_lookup() also handles the fixed entries and other > conditions, which do not have to be repeated. This patch moves > such code from __mtrr_type_lookup() to mtrr_type_lookup(). > > This patch also changes the 'else if (start < 0x1000000)', > which checks a fixed range but has an extra zero in the address, > to 'else' with no condition. > > Lastly, the patch updates the function headers to clarify the > return values and output argument. It also updates comments to > clarify that the repeating is necessary to handle overlaps with > the default type, since overlaps with multiple entries alone > can be handled without such repeating. > > There is no functional change in this patch. > > Signed-off-by: Toshi Kani > --- > arch/x86/kernel/cpu/mtrr/generic.c | 102 +++++++++++++++++++----------------- > 1 file changed, 53 insertions(+), 49 deletions(-) > > diff --git a/arch/x86/kernel/cpu/mtrr/generic.c b/arch/x86/kernel/cpu/mtrr/generic.c > index a82e370..ef34a4f 100644 > --- a/arch/x86/kernel/cpu/mtrr/generic.c > +++ b/arch/x86/kernel/cpu/mtrr/generic.c > @@ -102,12 +102,16 @@ static int check_type_overlap(u8 *prev, u8 *curr) > return 0; > } > > -/* > - * Error/Semi-error returns: > - * 0xFF - when MTRR is not enabled > - * *repeat == 1 implies [start:end] spanned across MTRR range and type returned > - * corresponds only to [start:*partial_end]. > - * Caller has to lookup again for [*partial_end:end]. > +/** > + * __mtrr_type_lookup - look up memory type in MTRR variable entries > + * > + * Return Value: > + * memory type - Matched memory type or the default memory type (unmatched) > + * > + * Output Argument: > + * repeat - Set to 1 when [start:end] spanned across MTRR range and type > + * returned corresponds only to [start:*partial_end]. Caller has > + * to lookup again for [*partial_end:end]. > */ > static u8 __mtrr_type_lookup(u64 start, u64 end, u64 *partial_end, int *repeat) > { > @@ -116,42 +120,10 @@ static u8 __mtrr_type_lookup(u64 start, u64 end, u64 *partial_end, int *repeat) > u8 prev_match, curr_match; > > *repeat = 0; > - if (!mtrr_state_set) > - return 0xFF; > - > - if (!mtrr_state.enabled) > - return 0xFF; > > /* Make end inclusive end, instead of exclusive */ > end--; > > - /* Look in fixed ranges. Just return the type as per start */ > - if (mtrr_state.have_fixed && (start < 0x100000)) { > - int idx; > - > - if (start < 0x80000) { > - idx = 0; > - idx += (start >> 16); > - return mtrr_state.fixed_ranges[idx]; > - } else if (start < 0xC0000) { > - idx = 1 * 8; > - idx += ((start - 0x80000) >> 14); > - return mtrr_state.fixed_ranges[idx]; > - } else if (start < 0x1000000) { > - idx = 3 * 8; > - idx += ((start - 0xC0000) >> 12); > - return mtrr_state.fixed_ranges[idx]; > - } > - } > - > - /* > - * Look in variable ranges > - * Look of multiple ranges matching this address and pick type > - * as per MTRR precedence > - */ > - if (!(mtrr_state.enabled & 2)) > - return mtrr_state.def_type; > - > prev_match = 0xFF; > for (i = 0; i < num_var_ranges; ++i) { > unsigned short start_state, end_state, inclusive; > @@ -180,7 +152,8 @@ static u8 __mtrr_type_lookup(u64 start, u64 end, u64 *partial_end, int *repeat) > * Return the type for first region and a pointer to > * the start of second region so that caller will > * lookup again on the second region. > - * Note: This way we handle multiple overlaps as well. > + * Note: This way we handle overlaps with multiple > + * entries and the default type properly. > */ > if (start_state) > *partial_end = base + get_mtrr_size(mask); > @@ -209,21 +182,18 @@ static u8 __mtrr_type_lookup(u64 start, u64 end, u64 *partial_end, int *repeat) > return curr_match; > } > > - if (mtrr_tom2) { > - if (start >= (1ULL<<32) && (end < mtrr_tom2)) > - return MTRR_TYPE_WRBACK; > - } > - > if (prev_match != 0xFF) > return prev_match; > > return mtrr_state.def_type; > } > > -/* > - * Returns the effective MTRR type for the region > - * Error return: > - * 0xFF - when MTRR is not enabled > +/** > + * mtrr_type_lookup - look up memory type in MTRR > + * > + * Return Values: > + * memory type - The effective MTRR type for the region > + * 0xFF - MTRR is disabled > */ > u8 mtrr_type_lookup(u64 start, u64 end) > { > @@ -231,12 +201,43 @@ u8 mtrr_type_lookup(u64 start, u64 end) > int repeat; > u64 partial_end; > > + if (!mtrr_state_set || !mtrr_state.enabled) > + return 0xFF; > + > + /* Look in fixed ranges. Just return the type as per start */ > + if (mtrr_state.have_fixed && (start < 0x100000)) { > + int idx; > + > + if (start < 0x80000) { > + idx = 0; > + idx += (start >> 16); > + return mtrr_state.fixed_ranges[idx]; > + } else if (start < 0xC0000) { > + idx = 1 * 8; > + idx += ((start - 0x80000) >> 14); > + return mtrr_state.fixed_ranges[idx]; > + } else { > + idx = 3 * 8; > + idx += ((start - 0xC0000) >> 12); > + return mtrr_state.fixed_ranges[idx]; > + } > + } So why not put this into a separate helper function - named mtrr_type_lookup_fixed()? It has little relation to variable ranges. > + > + /* > + * Look in variable ranges > + * Look of multiple ranges matching this address and pick type > + * as per MTRR precedence > + */ > + if (!(mtrr_state.enabled & 2)) > + return mtrr_state.def_type; > + > type = __mtrr_type_lookup(start, end, &partial_end, &repeat); And this then should be named mtrr_type_lookup_variable() or so? Thanks, Ingo From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1755409AbbCMNyF (ORCPT ); Fri, 13 Mar 2015 09:54:05 -0400 Received: from g9t5009.houston.hp.com ([15.240.92.67]:36689 "EHLO g9t5009.houston.hp.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1751209AbbCMNyB (ORCPT ); Fri, 13 Mar 2015 09:54:01 -0400 Message-ID: <1426254791.17007.451.camel@misato.fc.hp.com> Subject: Re: [PATCH v2 3/4] mtrr, x86: Clean up mtrr_type_lookup() From: Toshi Kani To: Ingo Molnar Cc: "akpm@linux-foundation.org" , "hpa@zytor.com" , "tglx@linutronix.de" , "mingo@redhat.com" , "linux-mm@kvack.org" , "x86@kernel.org" , "linux-kernel@vger.kernel.org" , "dave.hansen@intel.com" , "Elliott, Robert (Server Storage)" , "pebolle@tiscali.nl" Date: Fri, 13 Mar 2015 07:53:11 -0600 In-Reply-To: <20150313123722.GA4152@gmail.com> References: <1426180690-24234-1-git-send-email-toshi.kani@hp.com> <1426180690-24234-4-git-send-email-toshi.kani@hp.com> <20150313123722.GA4152@gmail.com> Content-Type: text/plain; charset="UTF-8" X-Mailer: Evolution 3.10.4 (3.10.4-4.fc20) Mime-Version: 1.0 Content-Transfer-Encoding: 7bit Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org On Fri, 2015-03-13 at 12:37 +0000, Ingo Molnar wrote: > * Toshi Kani wrote: : > > + /* Look in fixed ranges. Just return the type as per start */ > > + if (mtrr_state.have_fixed && (start < 0x100000)) { > > + int idx; > > + > > + if (start < 0x80000) { > > + idx = 0; > > + idx += (start >> 16); > > + return mtrr_state.fixed_ranges[idx]; > > + } else if (start < 0xC0000) { > > + idx = 1 * 8; > > + idx += ((start - 0x80000) >> 14); > > + return mtrr_state.fixed_ranges[idx]; > > + } else { > > + idx = 3 * 8; > > + idx += ((start - 0xC0000) >> 12); > > + return mtrr_state.fixed_ranges[idx]; > > + } > > + } > > So why not put this into a separate helper function - named > mtrr_type_lookup_fixed()? It has little relation to variable ranges. Sounds good. I will update as suggested. > > + > > + /* > > + * Look in variable ranges > > + * Look of multiple ranges matching this address and pick type > > + * as per MTRR precedence > > + */ > > + if (!(mtrr_state.enabled & 2)) > > + return mtrr_state.def_type; > > + > > type = __mtrr_type_lookup(start, end, &partial_end, &repeat); > > And this then should be named mtrr_type_lookup_variable() or so? Will do as well. I will send out a new version today since I won't be able to update the patchset next week. Thanks, -Toshi