[PATCH v3 0/2] x86, mtrr: fixs for mtrr_cleanup

[PATCH v3 0/2] x86, mtrr: fixs for mtrr_cleanup

[Yinghai Lu]

Joshua reported: Commit cd7b304dfaf1 (x86, range: fix missing merge
during add range) broke mtrr cleanup on his setup in 3.9.5.
corresponding commit in upstream is fbe06b7bae7c.

  *BAD*gran_size: 64K chunk_size: 16M num_reg: 6 lose cover RAM: -0G

https://bugzilla.kernel.org/show_bug.cgi?id=59491

Fix it from two places, so late mtrr cleanup side and range add side.

  x86, mtrr: Fix original mtrr range get for mtrr_cleanup
  range: Do not add new blank slot with add_range_with_merge

Please apply them for v3.10.

Thanks

Yinghai

[PATCH v3 1/2] x86, mtrr: Fix original mtrr range get for mtrr_cleanup

[Yinghai Lu]

Joshua reported: Commit cd7b304dfaf1 (x86, range: fix missing merge
during add range) broke mtrr cleanup on his setup in 3.9.5.
corresponding commit in upstream is fbe06b7bae7c.

  *BAD*gran_size: 64K chunk_size: 16M num_reg: 6 lose cover RAM: -0G

https://bugzilla.kernel.org/show_bug.cgi?id=59491

So it rejects new var mtrr layout.

It turns out we have some problem with initial mtrr range retrievel.
current sequence is:
	x86_get_mtrr_mem_range
		==> bunchs of add_range_with_merge
		==> bunchs of subract_range
		==> clean_sort_range
	add_range_with_merge for [0,1M)
	sort_range()

add_range_with_merge could have blank slots, so we can not just
sort only, that will have final result have extra blank slot in head.

So move that calling add_range_with_merge for [0,1M), with that we
could avoid extra clean_sort_range calling.

Reported-by: Joshua Covington <joshuacov@googlemail.com>
Tested-by: Joshua Covington <joshuacov@googlemail.com>
Signed-off-by: Yinghai Lu <yinghai@kernel.org>
Cc: <stable@vger.kernel.org> v3.9

---
 arch/x86/kernel/cpu/mtrr/cleanup.c |    8 ++++----
 1 file changed, 4 insertions(+), 4 deletions(-)

Index: linux-2.6/arch/x86/kernel/cpu/mtrr/cleanup.c
===================================================================
--- linux-2.6.orig/arch/x86/kernel/cpu/mtrr/cleanup.c
+++ linux-2.6/arch/x86/kernel/cpu/mtrr/cleanup.c
@@ -714,15 +714,15 @@ int __init mtrr_cleanup(unsigned address
 	if (mtrr_tom2)
 		x_remove_size = (mtrr_tom2 >> PAGE_SHIFT) - x_remove_base;
 
-	nr_range = x86_get_mtrr_mem_range(range, 0, x_remove_base, x_remove_size);
 	/*
 	 * [0, 1M) should always be covered by var mtrr with WB
 	 * and fixed mtrrs should take effect before var mtrr for it:
 	 */
-	nr_range = add_range_with_merge(range, RANGE_NUM, nr_range, 0,
+	nr_range = add_range_with_merge(range, RANGE_NUM, 0, 0,
 					1ULL<<(20 - PAGE_SHIFT));
-	/* Sort the ranges: */
-	sort_range(range, nr_range);
+	/* add from var mtrr at last */
+	nr_range = x86_get_mtrr_mem_range(range, nr_range,
+					  x_remove_base, x_remove_size);
 
 	range_sums = sum_ranges(range, nr_range);
 	printk(KERN_INFO "total RAM covered: %ldM\n",

Re: [PATCH v3 1/2] x86, mtrr: Fix original mtrr range get for mtrr_cleanup

[Josh Boyer]

On Thu, Jun 13, 2013 at 4:17 PM, Yinghai Lu <yinghai@kernel.org> wrote:
> Joshua reported: Commit cd7b304dfaf1 (x86, range: fix missing merge
> during add range) broke mtrr cleanup on his setup in 3.9.5.
> corresponding commit in upstream is fbe06b7bae7c.
>
>   *BAD*gran_size: 64K chunk_size: 16M num_reg: 6 lose cover RAM: -0G
>
> https://bugzilla.kernel.org/show_bug.cgi?id=59491
>
> So it rejects new var mtrr layout.
>
> It turns out we have some problem with initial mtrr range retrievel.
> current sequence is:
>         x86_get_mtrr_mem_range
>                 ==> bunchs of add_range_with_merge
>                 ==> bunchs of subract_range
>                 ==> clean_sort_range
>         add_range_with_merge for [0,1M)
>         sort_range()
>
> add_range_with_merge could have blank slots, so we can not just
> sort only, that will have final result have extra blank slot in head.
>
> So move that calling add_range_with_merge for [0,1M), with that we
> could avoid extra clean_sort_range calling.
>
> Reported-by: Joshua Covington <joshuacov@googlemail.com>
> Tested-by: Joshua Covington <joshuacov@googlemail.com>
> Signed-off-by: Yinghai Lu <yinghai@kernel.org>
> Cc: <stable@vger.kernel.org> v3.9

We're at -rc6 now and this and it's companion aren't in Linus' tree,
which means the broken 3.9.y stable series doesn't have them either.
It's been posted for a while now, and v3 was really a minor cleanup.
Is there some reason this is still not merged?  We've been carrying v2
in Fedora for a bit now and it's cleared up the issue for a number of
people.

josh

[tip:x86/urgent] x86, mtrr: Fix original mtrr range get for mtrr_cleanup

[tip-bot for Yinghai Lu]

Commit-ID:  d8d386c10630d8f7837700f4c466443d49e12cc0
Gitweb:     http://git.kernel.org/tip/d8d386c10630d8f7837700f4c466443d49e12cc0
Author:     Yinghai Lu <yinghai@kernel.org>
AuthorDate: Thu, 13 Jun 2013 13:17:01 -0700
Committer:  H. Peter Anvin <hpa@linux.intel.com>
CommitDate: Tue, 18 Jun 2013 11:32:02 -0500

x86, mtrr: Fix original mtrr range get for mtrr_cleanup

Joshua reported: Commit cd7b304dfaf1 (x86, range: fix missing merge
during add range) broke mtrr cleanup on his setup in 3.9.5.
corresponding commit in upstream is fbe06b7bae7c.

  *BAD*gran_size: 64K chunk_size: 16M num_reg: 6 lose cover RAM: -0G

https://bugzilla.kernel.org/show_bug.cgi?id=59491

So it rejects new var mtrr layout.

It turns out we have some problem with initial mtrr range retrieval.
The current sequence is:
	x86_get_mtrr_mem_range
		==> bunchs of add_range_with_merge
		==> bunchs of subract_range
		==> clean_sort_range
	add_range_with_merge for [0,1M)
	sort_range()

add_range_with_merge could have blank slots, so we can not just
sort only, that will have final result have extra blank slot in head.

So move that calling add_range_with_merge for [0,1M), with that we
could avoid extra clean_sort_range calling.

Reported-by: Joshua Covington <joshuacov@googlemail.com>
Tested-by: Joshua Covington <joshuacov@googlemail.com>
Signed-off-by: Yinghai Lu <yinghai@kernel.org>
Link: http://lkml.kernel.org/r/1371154622-8929-2-git-send-email-yinghai@kernel.org
Cc: <stable@vger.kernel.org> v3.9
Signed-off-by: H. Peter Anvin <hpa@linux.intel.com>
---
 arch/x86/kernel/cpu/mtrr/cleanup.c | 8 ++++----
 1 file changed, 4 insertions(+), 4 deletions(-)

diff --git a/arch/x86/kernel/cpu/mtrr/cleanup.c b/arch/x86/kernel/cpu/mtrr/cleanup.c
index 35ffda5..5f90b85 100644
--- a/arch/x86/kernel/cpu/mtrr/cleanup.c
+++ b/arch/x86/kernel/cpu/mtrr/cleanup.c
@@ -714,15 +714,15 @@ int __init mtrr_cleanup(unsigned address_bits)
 	if (mtrr_tom2)
 		x_remove_size = (mtrr_tom2 >> PAGE_SHIFT) - x_remove_base;
 
-	nr_range = x86_get_mtrr_mem_range(range, 0, x_remove_base, x_remove_size);
 	/*
 	 * [0, 1M) should always be covered by var mtrr with WB
 	 * and fixed mtrrs should take effect before var mtrr for it:
 	 */
-	nr_range = add_range_with_merge(range, RANGE_NUM, nr_range, 0,
+	nr_range = add_range_with_merge(range, RANGE_NUM, 0, 0,
 					1ULL<<(20 - PAGE_SHIFT));
-	/* Sort the ranges: */
-	sort_range(range, nr_range);
+	/* add from var mtrr at last */
+	nr_range = x86_get_mtrr_mem_range(range, nr_range,
+					  x_remove_base, x_remove_size);
 
 	range_sums = sum_ranges(range, nr_range);
 	printk(KERN_INFO "total RAM covered: %ldM\n",

[PATCH v3 2/2] range: Do not add new blank slot with add_range_with_merge

[Yinghai Lu]

Joshua reported: Commit cd7b304dfaf1 (x86, range: fix missing merge
during add range) broke mtrr cleanup on his setup in 3.9.5.
corresponding commit in upstream is fbe06b7bae7c.

The reason is add_range_with_merge could generate blank spot.

We could avoid that by searching new expanded start/end, that
new range should include all connected ranges in range array.
At last add the new expanded start/end to the range array.
Also move up left array so do not add new blank slot in the
range array.

-v2: move left array to avoid enhance add_range()
-v3: include fix from Joshua about memmove declaring when
     DYN_DEBUG is used.

Reported-by: Joshua Covington <joshuacov@googlemail.com>
Tested-by: Joshua Covington <joshuacov@googlemail.com>
Signed-off-by: Yinghai Lu <yinghai@kernel.org>
Cc: <stable@vger.kernel.org> v3.9

---
 kernel/range.c |   23 ++++++++++++-----------
 1 file changed, 12 insertions(+), 11 deletions(-)

Index: linux-2.6/kernel/range.c
===================================================================
--- linux-2.6.orig/kernel/range.c
+++ linux-2.6/kernel/range.c
@@ -4,7 +4,7 @@
 #include <linux/kernel.h>
 #include <linux/init.h>
 #include <linux/sort.h>
-
+#include <linux/string.h>
 #include <linux/range.h>
 
 int add_range(struct range *range, int az, int nr_range, u64 start, u64 end)
@@ -32,9 +32,8 @@ int add_range_with_merge(struct range *r
 	if (start >= end)
 		return nr_range;
 
-	/* Try to merge it with old one: */
+	/* get new start/end: */
 	for (i = 0; i < nr_range; i++) {
-		u64 final_start, final_end;
 		u64 common_start, common_end;
 
 		if (!range[i].end)
@@ -45,14 +44,16 @@ int add_range_with_merge(struct range *r
 		if (common_start > common_end)
 			continue;
 
-		final_start = min(range[i].start, start);
-		final_end = max(range[i].end, end);
-
-		/* clear it and add it back for further merge */
-		range[i].start = 0;
-		range[i].end =  0;
-		return add_range_with_merge(range, az, nr_range,
-			final_start, final_end);
+		/* new start/end, will add it back at last */
+		start = min(range[i].start, start);
+		end = max(range[i].end, end);
+
+		memmove(&range[i], &range[i + 1],
+			(nr_range - (i + 1)) * sizeof(range[i]));
+		range[nr_range - 1].start = 0;
+		range[nr_range - 1].end   = 0;
+		nr_range--;
+		i--;
 	}
 
 	/* Need to add it: */

[tip:x86/urgent] range: Do not add new blank slot with add_range_with_merge

[tip-bot for Yinghai Lu]

Commit-ID:  0541881502a1276149889fe468662ff6a8fc8f6d
Gitweb:     http://git.kernel.org/tip/0541881502a1276149889fe468662ff6a8fc8f6d
Author:     Yinghai Lu <yinghai@kernel.org>
AuthorDate: Thu, 13 Jun 2013 13:17:02 -0700
Committer:  H. Peter Anvin <hpa@linux.intel.com>
CommitDate: Tue, 18 Jun 2013 11:32:10 -0500

range: Do not add new blank slot with add_range_with_merge

Joshua reported: Commit cd7b304dfaf1 (x86, range: fix missing merge
during add range) broke mtrr cleanup on his setup in 3.9.5.
corresponding commit in upstream is fbe06b7bae7c.

The reason is add_range_with_merge could generate blank spot.

We could avoid that by searching new expanded start/end, that
new range should include all connected ranges in range array.
At last add the new expanded start/end to the range array.
Also move up left array so do not add new blank slot in the
range array.

-v2: move left array to avoid enhance add_range()
-v3: include fix from Joshua about memmove declaring when
     DYN_DEBUG is used.

Reported-by: Joshua Covington <joshuacov@googlemail.com>
Tested-by: Joshua Covington <joshuacov@googlemail.com>
Signed-off-by: Yinghai Lu <yinghai@kernel.org>
Link: http://lkml.kernel.org/r/1371154622-8929-3-git-send-email-yinghai@kernel.org
Cc: <stable@vger.kernel.org> v3.9
Signed-off-by: H. Peter Anvin <hpa@linux.intel.com>
---
 kernel/range.c | 21 +++++++++++----------
 1 file changed, 11 insertions(+), 10 deletions(-)

diff --git a/kernel/range.c b/kernel/range.c
index eb911db..322ea8e 100644
--- a/kernel/range.c
+++ b/kernel/range.c
@@ -4,7 +4,7 @@
 #include <linux/kernel.h>
 #include <linux/init.h>
 #include <linux/sort.h>
-
+#include <linux/string.h>
 #include <linux/range.h>
 
 int add_range(struct range *range, int az, int nr_range, u64 start, u64 end)
@@ -32,9 +32,8 @@ int add_range_with_merge(struct range *range, int az, int nr_range,
 	if (start >= end)
 		return nr_range;
 
-	/* Try to merge it with old one: */
+	/* get new start/end: */
 	for (i = 0; i < nr_range; i++) {
-		u64 final_start, final_end;
 		u64 common_start, common_end;
 
 		if (!range[i].end)
@@ -45,14 +44,16 @@ int add_range_with_merge(struct range *range, int az, int nr_range,
 		if (common_start > common_end)
 			continue;
 
-		final_start = min(range[i].start, start);
-		final_end = max(range[i].end, end);
+		/* new start/end, will add it back at last */
+		start = min(range[i].start, start);
+		end = max(range[i].end, end);
 
-		/* clear it and add it back for further merge */
-		range[i].start = 0;
-		range[i].end =  0;
-		return add_range_with_merge(range, az, nr_range,
-			final_start, final_end);
+		memmove(&range[i], &range[i + 1],
+			(nr_range - (i + 1)) * sizeof(range[i]));
+		range[nr_range - 1].start = 0;
+		range[nr_range - 1].end   = 0;
+		nr_range--;
+		i--;
 	}
 
 	/* Need to add it: */

Re: [PATCH v3 0/2] x86, mtrr: fixs for mtrr_cleanup

[H. Peter Anvin]

On 06/13/2013 03:17 PM, Yinghai Lu wrote:
> Joshua reported: Commit cd7b304dfaf1 (x86, range: fix missing merge
> during add range) broke mtrr cleanup on his setup in 3.9.5.
> corresponding commit in upstream is fbe06b7bae7c.
> 
>   *BAD*gran_size: 64K chunk_size: 16M num_reg: 6 lose cover RAM: -0G
> 
> https://bugzilla.kernel.org/show_bug.cgi?id=59491
> 
> Fix it from two places, so late mtrr cleanup side and range add side.
> 
>   x86, mtrr: Fix original mtrr range get for mtrr_cleanup
>   range: Do not add new blank slot with add_range_with_merge
> 
> Please apply them for v3.10.
> 

On this subject: what do we need to happen to be able to dump the MTRR
cleanup stuff?

	-hpa

Re: [PATCH v3 0/2] x86, mtrr: fixs for mtrr_cleanup

[Yinghai Lu]

On Tue, Jun 18, 2013 at 9:29 AM, H. Peter Anvin <hpa@zytor.com> wrote:
>
> On this subject: what do we need to happen to be able to dump the MTRR
> cleanup stuff?

the old x server like to add Write-combining entry in MTRR.
looks like drm for mga200 is adding that too ?

We need to make them to use PAT instead.

Yinghai

Re: [PATCH v3 0/2] x86, mtrr: fixs for mtrr_cleanup

[H. Peter Anvin]

On 06/18/2013 12:28 PM, Yinghai Lu wrote:
> On Tue, Jun 18, 2013 at 9:29 AM, H. Peter Anvin <hpa@zytor.com> wrote:
>>
>> On this subject: what do we need to happen to be able to dump the MTRR
>> cleanup stuff?
> 
> the old x server like to add Write-combining entry in MTRR.
> looks like drm for mga200 is adding that too ?
>

"Old X server" here has been a long time now.  I need to dig into the
MGA200 I guess...

> We need to make them to use PAT instead.

Yes.

	-hpa

Re: [PATCH v3 0/2] x86, mtrr: fixs for mtrr_cleanup

[Yinghai Lu]

On Tue, Jun 18, 2013 at 10:33 AM, H. Peter Anvin <hpa@zytor.com> wrote:
> On 06/18/2013 12:28 PM, Yinghai Lu wrote:
>> On Tue, Jun 18, 2013 at 9:29 AM, H. Peter Anvin <hpa@zytor.com> wrote:
>>>
>>> On this subject: what do we need to happen to be able to dump the MTRR
>>> cleanup stuff?
>>
>> the old x server like to add Write-combining entry in MTRR.
>> looks like drm for mga200 is adding that too ?
>>
>
> "Old X server" here has been a long time now.  I need to dig into the
> MGA200 I guess...
>
>> We need to make them to use PAT instead.

looks that we have more mtrr_add calling:

drivers/gpu/drm/ast/ast_ttm.c:  ast->fb_mtrr =
drm_mtrr_add(pci_resource_start(dev->pdev, 0),
drivers/gpu/drm/cirrus/cirrus_ttm.c:    cirrus->fb_mtrr =
drm_mtrr_add(pci_resource_start(dev->pdev, 0),
drivers/gpu/drm/drm_bufs.c:                             map->mtrr =
mtrr_add(map->offset, map->size,
drivers/gpu/drm/drm_pci.c:
mtrr_add(dev->agp->agp_info.aper_base,
drivers/gpu/drm/i915/i915_dma.c:        dev_priv->mm.gtt_mtrr =
mtrr_add(base, size, MTRR_TYPE_WRCOMB, 1);
drivers/gpu/drm/mgag200/mgag200_ttm.c:  mdev->fb_mtrr =
drm_mtrr_add(pci_resource_start(dev->pdev, 0),
drivers/gpu/drm/nouveau/nouveau_ttm.c:  drm->ttm.mtrr =
drm_mtrr_add(pci_resource_start(dev->pdev, 1),
drivers/gpu/drm/radeon/radeon_object.c:         rdev->mc.vram_mtrr =
mtrr_add(rdev->mc.aper_base, rdev->mc.aper_size,
drivers/gpu/drm/savage/savage_bci.c:
drm_mtrr_add(dev_priv->mtrr[0].base,
drivers/gpu/drm/savage/savage_bci.c:
drm_mtrr_add(dev_priv->mtrr[1].base,
drivers/gpu/drm/savage/savage_bci.c:
drm_mtrr_add(dev_priv->mtrr[2].base,
drivers/gpu/drm/savage/savage_bci.c:
drm_mtrr_add(dev_priv->mtrr[0].base,
drivers/gpu/drm/vmwgfx/vmwgfx_drv.c:    dev_priv->mmio_mtrr =
drm_mtrr_add(dev_priv->mmio_start,
drivers/infiniband/hw/ipath/ipath_wc_x86_64.c:          cookie =
mtrr_add(pioaddr, piolen, MTRR_TYPE_WRCOMB, 0);
drivers/infiniband/hw/ipath/ipath_wc_x86_64.c:
          "mtrr_add()  WC for PIO bufs "
drivers/infiniband/hw/qib/qib_wc_x86_64.c:              cookie =
mtrr_add(pioaddr, piolen, MTRR_TYPE_WRCOMB, 0);
drivers/infiniband/hw/qib/qib_wc_x86_64.c:
          "mtrr_add()  WC for PIO bufs failed (%d)\n",
drivers/media/pci/ivtv/ivtvfb.c:                if
(mtrr_add(oi->fb_start_aligned_physaddr,
drivers/message/fusion/mptbase.c:               ioc->mtrr_reg =
mtrr_add(ioc->req_frames_dma,
drivers/net/ethernet/myricom/myri10ge/myri10ge.c:       mgp->mtrr =
mtrr_add(mgp->iomem_base, mgp->board_span,

will be in drm:

drivers/staging/xgifb/XGI_main_26.c:    xgifb_info->mtrr =
mtrr_add(xgifb_info->video_base,

video:

drivers/video/arkfb.c:          par->mtrr_reg =
mtrr_add(info->fix.smem_start, info->fix.smem_len, MTRR_TYPE_WRCOMB,
1);
drivers/video/aty/aty128fb.c:           par->mtrr.vram =
mtrr_add(info->fix.smem_start,
drivers/video/aty/atyfb_base.c:         par->mtrr_aper =
mtrr_add(par->res_start, par->res_size,
drivers/video/aty/atyfb_base.c:                 par->mtrr_reg =
mtrr_add(par->res_start + 0x800000 -
drivers/video/aty/radeon_base.c:        rinfo->mtrr_hdl = nomtrr ? -1
: mtrr_add(rinfo->fb_base_phys,
drivers/video/gbefb.c:  mtrr_add(gbe_mem_phys, gbe_mem_size,
MTRR_TYPE_WRCOMB, 1);
drivers/video/i740fb.c:         par->mtrr_reg = mtrr_add(info->fix.smem_start,
drivers/video/i810/i810_main.h: par->mtrr_reg = mtrr_add((u32)
par->aperture.physical,
drivers/video/intelfb/intelfbdrv.c:     dinfo->mtrr_reg =
mtrr_add(dinfo->aperture.physical,
drivers/video/kyro/fbdev.c:                     mtrr_add(kyro_fix.smem_start,
drivers/video/matrox/matroxfb_base.c:           minfo->mtrr.vram =
mtrr_add(video_base_phys, minfo->video.len, MTRR_TYPE_WRCOMB, 1);
drivers/video/neofb.c:          mtrr_add(info->fix.smem_start,
pci_resource_len(dev, 0),
drivers/video/nvidia/nvidia.c:          par->mtrr.vram =
mtrr_add(nvidiafb_fix.smem_start,
drivers/video/pm2fb.c:                  mtrr_add(pm2fb_fix.smem_start,
drivers/video/pm3fb.c:          par->mtrr_handle =
mtrr_add(pm3fb_fix.smem_start,
drivers/video/riva/fbdev.c:             default_par->mtrr.vram =
mtrr_add(rivafb_fix.smem_start,
drivers/video/s3fb.c:           par->mtrr_reg =
mtrr_add(info->fix.smem_start, info->fix.smem_len, MTRR_TYPE_WRCOMB,
1);
drivers/video/savage/savagefb_driver.c: par->video.mtrr =
mtrr_add(par->video.pbase, video_len,
drivers/video/sgivwfb.c:        mtrr_add(sgivwfb_mem_phys,
sgivwfb_mem_size, MTRR_TYPE_WRCOMB, 1);
drivers/video/sis/sis_main.c:           ivideo->mtrr =
mtrr_add(ivideo->video_base, ivideo->video_size,
drivers/video/tdfxfb.c:static inline int mtrr_add(unsigned long base,
unsigned long size,
drivers/video/tdfxfb.c:                 mtrr_add(info->fix.smem_start,
info->fix.smem_len,
drivers/video/uvesafb.c:                                rc =
mtrr_add(info->fix.smem_start,
drivers/video/vesafb.c:                         rc =
mtrr_add(vesafb_fix.smem_start, temp_size,
drivers/video/vt8623fb.c:               par->mtrr_reg =
mtrr_add(info->fix.smem_start, info->fix.smem_len, MTRR_TYPE_WRCOMB,
1);