WARNING - OLD ARCHIVES

This is an archived copy of the Xen.org mailing list, which we have preserved to ensure that existing links to archives are not broken. The live archive, which contains the latest emails, can be found at http://lists.xen.org/
   
 
 
Xen 
 
Home Products Support Community News
 
   
 

xen-devel

Re: [Xen-devel] [PATCH] MTRR: clear DramModEn bit of sys_cfg MSR

To: Keir Fraser <keir@xxxxxxx>
Subject: Re: [Xen-devel] [PATCH] MTRR: clear DramModEn bit of sys_cfg MSR
From: Wei Huang <wei.huang2@xxxxxxx>
Date: Tue, 5 Apr 2011 12:26:42 -0500
Cc: "'xen-devel@xxxxxxxxxxxxxxxxxxx'" <xen-devel@xxxxxxxxxxxxxxxxxxx>
Delivery-date: Tue, 05 Apr 2011 10:29:45 -0700
Envelope-to: www-data@xxxxxxxxxxxxxxxxxxx
In-reply-to: <C9C0C0D4.2C500%keir@xxxxxxx>
List-help: <mailto:xen-devel-request@lists.xensource.com?subject=help>
List-id: Xen developer discussion <xen-devel.lists.xensource.com>
List-post: <mailto:xen-devel@lists.xensource.com>
List-subscribe: <http://lists.xensource.com/mailman/listinfo/xen-devel>, <mailto:xen-devel-request@lists.xensource.com?subject=subscribe>
List-unsubscribe: <http://lists.xensource.com/mailman/listinfo/xen-devel>, <mailto:xen-devel-request@lists.xensource.com?subject=unsubscribe>
References: <C9C0C0D4.2C500%keir@xxxxxxx>
Sender: xen-devel-bounces@xxxxxxxxxxxxxxxxxxx
User-agent: Mozilla/5.0 (X11; U; Linux i686 (x86_64); en-US; rv:1.9.2.15) Gecko/20110303 Thunderbird/3.1.9
Sorry for the indention issue... I agree that it is better to move check-and_fixup code to init_amd() function. See the new patch.

My understanding is that RdMem and WrMem are never going to become 1: BIOS are supposed to set sys_cfg[DramModEn] bit to 0 before OS takes over. As a result, RdMem and WrMem are read as 0 in fixed MTRRs. Unless Xen turn these bits to 1 (which I don't see from the code), k8_enable_fixed_iorrs() is useless. My 2nd patch removes k8_enable_fixed_iorrs(). If you have concern, we don't have to apply this patch. But I don't think we shouldn't move it to amd.c. k8_enable_fixed_iorrs() is unrelated in that file.

Thanks,
-Wei

On 04/05/2011 06:51 AM, Keir Fraser wrote:
On 04/04/2011 23:23, "Wei Huang"<wei.huang2@xxxxxxx>  wrote:

Some buggy BIOS might set sys_cfg DramModEn bit to 1, which can cause
unexpected behavior on AMD platforms. This patch clears DramModEn bit.
The patch was derived from upstream kernel patch (see
https://patchwork.kernel.org/patch/11425/).
This patch also removes k8_enable_fixed_iorrs(), and it's not clear why.
That would at least belong in a separate patch, but I would think we don't
want to delete that code anyway.

The indentation is wrong (spaces in a file that is hard-tab-indented). And
the printk is probably unnecessary -- at most you should print it once
rather than potentially for every core brought up.

But further, I don't see why you need to hang off {get,set}_fixed_ranges at
all. Why not do this check-and-fixup in cpu/amd.c:init_amd()? It's a handy
early-cpu-bringup amd-specific function which is rather designed ofr this
kind of purpose. The k8_enable_fixed_iorrs() work could better be done in
the same place, too (perhaps move it in a separate patch?).

  -- Keir

Signed-off-by: Wei Huang<wei.huang2@xxxxxxx>




_______________________________________________
Xen-devel mailing list
Xen-devel@xxxxxxxxxxxxxxxxxxx
http://lists.xensource.com/xen-devel



Attachment: amd_fix_syscfg.txt
Description: amd_fix_syscfg.txt

Attachment: amd_remove_k8_enable_fixed_iorrs.txt
Description: amd_remove_k8_enable_fixed_iorrs.txt

_______________________________________________
Xen-devel mailing list
Xen-devel@xxxxxxxxxxxxxxxxxxx
http://lists.xensource.com/xen-devel