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

[Wei Huang <wei.huang2@xxxxxxx>]

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