On Mon, 2015-05-18 at 15:33 +0200, Borislav Petkov wrote:
> On Fri, May 15, 2015 at 12:23:57PM -0600, Toshi Kani wrote:
> > 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 covered
> > 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 <toshi.kani@xxxxxx>
> > ---
> > arch/x86/include/asm/mtrr.h | 4 ++--
> > arch/x86/kernel/cpu/mtrr/generic.c | 37 ++++++++++++++++++++++++++----------
> > arch/x86/mm/pat.c | 4 ++--
> > arch/x86/mm/pgtable.c | 33 ++++++++++++++++++++------------
> > 4 files changed, 52 insertions(+), 26 deletions(-)
:
>
> All applied,
Great!
> I reformatted the comments in this last one a bit and made
> the warning message hopefully a bit more descriptive:
I have a few comments below.
> diff --git a/arch/x86/mm/pgtable.c b/arch/x86/mm/pgtable.c
> index c30f9819786b..f1894daa79ee 100644
> --- a/arch/x86/mm/pgtable.c
> +++ b/arch/x86/mm/pgtable.c
> @@ -566,19 +566,24 @@ 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 4KiB granularity. Therefore,
> - * this function does not set up a huge page when the range is covered
> - * by a non-WB type of MTRR. MTRR_TYPE_INVALID indicates that MTRR are
> - * disabled.
> + * MTRRs can override PAT memory types with 4KiB granularity. Therefore,
> + * this function sets up a huge page only if all of the following
> + * conditions are met:
It should be "if any of the following condition is met". Or, does NOT
setup if all of ...
> + *
> + * - MTRRs are disabled.
> + * - The range is mapped uniformly by an MTRR, i.e. the range is
> + * fully covered by a single MTRR entry or the default type.
> + * - The MTRR memory type is WB.
> *
> * Returns 1 on success and 0 on failure.
> */
> 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 != MTRR_TYPE_INVALID))
> + mtrr = mtrr_type_lookup(addr, addr + PUD_SIZE, &uniform);
> + if ((mtrr != MTRR_TYPE_INVALID) && (!uniform) &&
> + (mtrr != MTRR_TYPE_WRBACK))
> return 0;
>
> prot = pgprot_4k_2_large(prot);
> @@ -593,20 +598,28 @@ 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 4KiB granularity. Therefore,
> - * this function does not set up a huge page when the range is covered
> - * by a non-WB type of MTRR. MTRR_TYPE_INVALID indicates that MTRR are
> - * disabled.
> + * MTRRs can override PAT memory types with 4KiB granularity. Therefore,
> + * this function sets up a huge page only if all of the following
> + * conditions are met:
Ditto.
> + *
> + * - MTRR is disabled.
> + * - The range is mapped uniformly by an MTRR, i.e. the range is
> + * fully covered by a single MTRR entry or the default type.
> + * - The MTRR memory type is WB.
> *
> * Returns 1 on success and 0 on failure.
> */
> 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 != MTRR_TYPE_INVALID))
> + mtrr = mtrr_type_lookup(addr, addr + PMD_SIZE, &uniform);
> + if ((mtrr != MTRR_TYPE_INVALID) && (!uniform) &&
> + (mtrr != MTRR_TYPE_WRBACK)) {
> + pr_warn_once("%s: Cannot satisfy [mem %#010llx-%#010llx] with a huge-page mapping due to MTRR override.\n",
> + __func__, addr, addr + PMD_SIZE);
This new message looks good.
Thanks,
-Toshi
--
To unsubscribe, send a message with 'unsubscribe linux-mm' in
the body to majordomo@xxxxxxxxx. For more info on Linux MM,
see: http://www.linux-mm.org/ .
Don't email: <a href=mailto:"dont@xxxxxxxxx";> email@xxxxxxxxx </a>