From mboxrd@z Thu Jan 1 00:00:00 1970 From: Keir Fraser Subject: Re: [PATCH] MTRR: clear DramModEn bit of sys_cfg MSR Date: Tue, 05 Apr 2011 12:51:32 +0100 Message-ID: References: <4D9A44E2.2060103@amd.com> Mime-Version: 1.0 Content-Type: text/plain; charset="US-ASCII" Content-Transfer-Encoding: 7bit Return-path: In-Reply-To: <4D9A44E2.2060103@amd.com> List-Unsubscribe: , List-Post: List-Help: List-Subscribe: , Sender: xen-devel-bounces@lists.xensource.com Errors-To: xen-devel-bounces@lists.xensource.com To: Wei Huang , "'xen-devel@lists.xensource.com'" List-Id: xen-devel@lists.xenproject.org On 04/04/2011 23:23, "Wei Huang" 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 > > > > > _______________________________________________ > Xen-devel mailing list > Xen-devel@lists.xensource.com > http://lists.xensource.com/xen-devel