Re: [PATCH 2/2] x86/mtrr: Refactor PAT initialization code

[Toshi Kani]

On Thu, 2016-03-17 at 17:06 -0700, Luis R. Rodriguez wrote:
> On Mar 17, 2016 2:04 PM, "Toshi Kani" <toshi.kani@xxxxxxx> wrote:
> > 
> > On Wed, 2016-03-16 at 00:29 +0100, Luis R. Rodriguez wrote:
> > > On Tue, Mar 15, 2016 at 05:48:44PM -0600, Toshi Kani wrote:
> > > > On Tue, 2016-03-15 at 01:15 +0100, Luis R. Rodriguez wrote:
> > > > > On Fri, Mar 11, 2016 at 06:16:36PM -0700, Toshi Kani wrote:
> > > > > >Â
> > Â:
> > > > MTRR code does not have to be dead for the virtual machines with no
> > > > MTRR support.ÂÂThe code just needs to handle the disabled case
> > > > properly.ÂÂI consider this is part of 1) that the kernel keeps the
> > > > MTRR state as disabled.
> > > 
> > > For Xen it was possible to use PAT without any of the MTRR code
> > > running, I don't see why its needed then and why other virtual
> > > machines that only need PAT need it.
> > 
> > Virtual BIOS does not need MTRRs since it does not manage the platform.
> 
> Unless if in dom0 and if some of this purposely wants to be punted
> there to leverage existing kernel code. On the Xen thread I'm asking
> about the implications of that and how/if Xen should be doing. We can
> follow up on this there as its Xen specific.

I do not see any issue for Xen, but sure, we can discuss about Xen in a
separate thread.


Â:
> > > On x86 Linux code we now have ioremap_uc() that can't use MTRR behind
> > > the scenes, why would something like this on the BIOS not be
> > > possible? That ultimately uses set_pte_at(). What limitations are
> > > there on the BIOS that prevent us from just using strong UC for PAT
> > > on the BIOS?
> > 
> > Because it requires to run in virtual mode with page tables.
> 
> Ah... interesting... is UC really needed, what is the default? If the
> default is used would there be an issue ? Can such work be deferred to
> a later time ? It seems like a high burden to require on large piece
> of legacy architecture to just blow a fan.

The default cache attribute (i.e. ranges not covered by MTRRs) is specified
by the MTRR default type MSR.

Â:
> > > > > >Â
> > > MTRR has a bunch of junk that is outside of the scope of the generic
> > > procedure which I'd hope we can skip.
> > 
> > We can skip the part that modifies MTRR setup. I think that is the part
> > you think is a junk.
> 
> Sure, but the more we can avoid any of that code the better. For
> example consider the cleanup patch to increase the chunk size, do we
> even need the cleanup anymore ?

No, I do not think we need it now. ÂI think this cleanup was needed to
allocate more free slots to MTRRs. ÂWe do not need to care about the number
of free slots as long as the kernel does not insert any new entry to MTRRs.

Thanks,
-Toshi