[PATCH 4/4] kvm: bios: switch MTRRs to cover only the PCI range and default to WB

[PATCH 4/4] kvm: bios: switch MTRRs to cover only the PCI range and default to WB

[Alex Williamson]

kvm: bios: switch MTRRs to cover only the PCI range and default to WB

This matches how some bare metal machines report MTRRs and avoids
the problem of running out of MTRRs to cover all of RAM.

Signed-off-by: Alex Williamson <alex.williamson@hp.com>
---

 bios/rombios32.c |   24 ++++--------------------
 1 files changed, 4 insertions(+), 20 deletions(-)

diff --git a/bios/rombios32.c b/bios/rombios32.c
index f8edf18..592abf9 100755
--- a/bios/rombios32.c
+++ b/bios/rombios32.c
@@ -494,7 +494,6 @@ void setup_mtrr(void)
         uint8_t valb[8];
         uint64_t val;
     } u;
-    uint64_t vbase, vmask;
 
     mtrr_cap = rdmsr(MSR_MTRRcap);
     vcnt = mtrr_cap & 0xff;
@@ -521,25 +520,10 @@ void setup_mtrr(void)
     wrmsr_smp(MSR_MTRRfix4K_E8000, 0);
     wrmsr_smp(MSR_MTRRfix4K_F0000, 0);
     wrmsr_smp(MSR_MTRRfix4K_F8000, 0);
-    vbase = 0;
-    --vcnt; /* leave one mtrr for VRAM */
-    for (i = 0; i < vcnt && vbase < ram_size; ++i) {
-        vmask = (1ull << 40) - 1;
-        while (vbase + vmask + 1 > ram_size)
-            vmask >>= 1;
-        wrmsr_smp(MTRRphysBase_MSR(i), vbase | 6);
-        wrmsr_smp(MTRRphysMask_MSR(i), (~vmask & 0xfffffff000ull) | 0x800);
-        vbase += vmask + 1;
-    }
-    for (vbase = 1ull << 32; i < vcnt && vbase < ram_end; ++i) {
-        vmask = (1ull << 40) - 1;
-        while (vbase + vmask + 1 > ram_end)
-            vmask >>= 1;
-        wrmsr_smp(MTRRphysBase_MSR(i), vbase | 6);
-        wrmsr_smp(MTRRphysMask_MSR(i), (~vmask & 0xfffffff000ull) | 0x800);
-        vbase += vmask + 1;
-    }
-    wrmsr_smp(MSR_MTRRdefType, 0xc00);
+    /* Mark 3.5-4GB as UC, anything not specified defaults to WB */
+    wrmsr_smp(MTRRphysBase_MSR(0), 0xe0000000ull | 0);
+    wrmsr_smp(MTRRphysMask_MSR(0), ~(0x20000000ull - 1) | 0x800);
+    wrmsr_smp(MSR_MTRRdefType, 0xc06);
 }
 
 void ram_probe(void)

Re: [PATCH 4/4] kvm: bios: switch MTRRs to cover only the PCI range and default to WB

[Yang, Sheng]

On Friday 26 September 2008 01:52:29 Alex Williamson wrote:
> kvm: bios: switch MTRRs to cover only the PCI range and default to WB
>
> This matches how some bare metal machines report MTRRs and avoids
> the problem of running out of MTRRs to cover all of RAM.
>
> Signed-off-by: Alex Williamson <alex.williamson@hp.com>
> ---
>
>  bios/rombios32.c |   24 ++++--------------------
>  1 files changed, 4 insertions(+), 20 deletions(-)
>
> diff --git a/bios/rombios32.c b/bios/rombios32.c
> index f8edf18..592abf9 100755
> --- a/bios/rombios32.c
> +++ b/bios/rombios32.c
> @@ -494,7 +494,6 @@ void setup_mtrr(void)
>          uint8_t valb[8];
>          uint64_t val;
>      } u;
> -    uint64_t vbase, vmask;
>
>      mtrr_cap = rdmsr(MSR_MTRRcap);
>      vcnt = mtrr_cap & 0xff;
> @@ -521,25 +520,10 @@ void setup_mtrr(void)
>      wrmsr_smp(MSR_MTRRfix4K_E8000, 0);
>      wrmsr_smp(MSR_MTRRfix4K_F0000, 0);
>      wrmsr_smp(MSR_MTRRfix4K_F8000, 0);
> -    vbase = 0;
> -    --vcnt; /* leave one mtrr for VRAM */
> -    for (i = 0; i < vcnt && vbase < ram_size; ++i) {
> -        vmask = (1ull << 40) - 1;
> -        while (vbase + vmask + 1 > ram_size)
> -            vmask >>= 1;
> -        wrmsr_smp(MTRRphysBase_MSR(i), vbase | 6);
> -        wrmsr_smp(MTRRphysMask_MSR(i), (~vmask & 0xfffffff000ull) |
> 0x800); -        vbase += vmask + 1;
> -    }
> -    for (vbase = 1ull << 32; i < vcnt && vbase < ram_end; ++i) {
> -        vmask = (1ull << 40) - 1;
> -        while (vbase + vmask + 1 > ram_end)
> -            vmask >>= 1;
> -        wrmsr_smp(MTRRphysBase_MSR(i), vbase | 6);
> -        wrmsr_smp(MTRRphysMask_MSR(i), (~vmask & 0xfffffff000ull) |
> 0x800); -        vbase += vmask + 1;
> -    }
> -    wrmsr_smp(MSR_MTRRdefType, 0xc00);
> +    /* Mark 3.5-4GB as UC, anything not specified defaults to WB */
> +    wrmsr_smp(MTRRphysBase_MSR(0), 0xe0000000ull | 0);
> +    wrmsr_smp(MTRRphysMask_MSR(0), ~(0x20000000ull - 1) | 0x800);
> +    wrmsr_smp(MSR_MTRRdefType, 0xc06);
>  }
>

I think we should do a little more than just write msr to update mtrr.

Intel SDM 10.11.8 "MTRR consideration in MP Systems" define the procedure to 
modify MTRR msr in MP. Especially, step 4 enter no-fill cache mode(set CR0.CD 
bit and clean NW bit), step 12 re-enabled the caching(clear this two bits).

We based on these behaviors to detect MTRR update.

(Forgot to raise the bug to Avi, recalled it now...)
--
regards
Yang, Sheng

Re: [PATCH 4/4] kvm: bios: switch MTRRs to cover only the PCI range and default to WB

[Avi Kivity]

Yang, Sheng wrote:
> I think we should do a little more than just write msr to update mtrr.
>
> Intel SDM 10.11.8 "MTRR consideration in MP Systems" define the procedure to 
> modify MTRR msr in MP. Especially, step 4 enter no-fill cache mode(set CR0.CD 
> bit and clean NW bit), step 12 re-enabled the caching(clear this two bits).
>
> We based on these behaviors to detect MTRR update.
>
>   

Why not simply flush the mmu on an mtrr write?

(though of course I have no objection to doing what the manual says)

-- 
I have a truly marvellous patch that fixes the bug which this
signature is too narrow to contain.

Re: [PATCH 4/4] kvm: bios: switch MTRRs to cover only the PCI range and default to WB

[Zwane Mwaikambo]

On Sat, 27 Sep 2008, Avi Kivity wrote:

> Yang, Sheng wrote:
> > I think we should do a little more than just write msr to update mtrr.
> > 
> > Intel SDM 10.11.8 "MTRR consideration in MP Systems" define the procedure to
> > modify MTRR msr in MP. Especially, step 4 enter no-fill cache mode(set
> > CR0.CD bit and clean NW bit), step 12 re-enabled the caching(clear this two
> > bits).
> > 
> > We based on these behaviors to detect MTRR update.
> > 
> >   
> 
> Why not simply flush the mmu on an mtrr write?
> 
> (though of course I have no objection to doing what the manual says)

Detecting that condition is fine for operating systems which follow it, 
but some don't, including older Linux kernels :( Flushing on MTRR write, 
although being overzealous would be the most robust.

	Zwane

Re: [PATCH 4/4] kvm: bios: switch MTRRs to cover only the PCI range and default to WB

[Sheng Yang]

On Saturday 27 September 2008 21:55:33 Zwane Mwaikambo wrote:
> On Sat, 27 Sep 2008, Avi Kivity wrote:
> > Yang, Sheng wrote:
> > > I think we should do a little more than just write msr to update mtrr.
> > >
> > > Intel SDM 10.11.8 "MTRR consideration in MP Systems" define the
> > > procedure to modify MTRR msr in MP. Especially, step 4 enter no-fill
> > > cache mode(set CR0.CD bit and clean NW bit), step 12 re-enabled the
> > > caching(clear this two bits).
> > >
> > > We based on these behaviors to detect MTRR update.
> >
> > Why not simply flush the mmu on an mtrr write?
> >
> > (though of course I have no objection to doing what the manual says)
>
> Detecting that condition is fine for operating systems which follow it,
> but some don't, including older Linux kernels :( Flushing on MTRR write,
> although being overzealous would be the most robust.

OK, this trade off is reasonable, I will update the mtrr patch.

Hope we won't got problem in so early stage. :)
--
regards
Yang, Sheng