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/
Home Products Support Community News


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

To: Wei Huang <wei.huang2@xxxxxxx>
Subject: Re: [Xen-devel] [PATCH] MTRR: clear DramModEn bit of sys_cfg MSR
From: Keir Fraser <keir.xen@xxxxxxxxx>
Date: Tue, 05 Apr 2011 19:12:39 +0100
Cc: "'xen-devel@xxxxxxxxxxxxxxxxxxx'" <xen-devel@xxxxxxxxxxxxxxxxxxx>
Delivery-date: Tue, 05 Apr 2011 11:13:24 -0700
Dkim-signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=gmail.com; s=gamma; h=domainkey-signature:user-agent:date:subject:from:to:cc:message-id :thread-topic:thread-index:in-reply-to:mime-version:content-type :content-transfer-encoding; bh=0tZNUclQ1XsyA9N5Dxr+TlwpfNfG424D/lhXQPVo7oE=; b=XnChFDonFV1S1Y9HuoS5okpuNJ9t7nFkh1WAt+MLYzOxWT8tlE95EgQJ6KPIUTQ5h9 cbLnZt3XsLZc0nG6Mrbm7IpcpUDBLNnnQIPSQ4gNvSQ5ulWTcs9wUXtxKQYdG5lN/11X ynXHXl7er+XEjfOnvQIfhNXlY/KqIc/L+BvNk=
Domainkey-signature: a=rsa-sha1; c=nofws; d=gmail.com; s=gamma; h=user-agent:date:subject:from:to:cc:message-id:thread-topic :thread-index:in-reply-to:mime-version:content-type :content-transfer-encoding; b=hH+VKu378pU86LGOOcVoEf84sNjpbEqMy8xbXlL7IkAIxP8TX7ivRGdxVm/3zAUvq+ xXvKtimMwP74xB0r+EFD8IrKu6eky1nxtliBC+2uWwnoWmxorsRzU67vWTPCft1Tb/BY /Yh/XEr2B2+Cr/WUhrMTC2gNWrEHPna/Wcui8=
Envelope-to: www-data@xxxxxxxxxxxxxxxxxxx
In-reply-to: <4D9B50D2.8040900@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>
Sender: xen-devel-bounces@xxxxxxxxxxxxxxxxxxx
Thread-index: AcvzvQw+hdk/jyKW8ESfHNjvPIC0Eg==
Thread-topic: [Xen-devel] [PATCH] MTRR: clear DramModEn bit of sys_cfg MSR
User-agent: Microsoft-Entourage/
Looks good, thanks!

 -- Keir

On 05/04/2011 18:26, "Wei Huang" <wei.huang2@xxxxxxx> wrote:

> 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

Xen-devel mailing list

<Prev in Thread] Current Thread [Next in Thread>