Re: PAT & pmap(9) changes (was Re: CVS commit: src)

[Christoph Egger <Christoph_Egger%gmx.de@localhost>]

On Wednesday 07 July 2010 22:49:51 David Young wrote:
> On Tue, Jul 06, 2010 at 08:50:36PM +0000, Christoph Egger wrote:
> > Module Name:        src
> > Committed By:       cegger
> > Date:               Tue Jul  6 20:50:36 UTC 2010
> >
> > Modified Files:
> >     src/share/man/man9: pmap.9
> >     src/sys/arch/amd64/include: pte.h
> >     src/sys/arch/hppa/include: pmap.h
> >     src/sys/arch/i386/include: pte.h
> >     src/sys/arch/mips/include: pmap.h
> >     src/sys/arch/mips/mips: pmap.c
> >     src/sys/arch/sgimips/sgimips: bus.c
> >     src/sys/arch/x86/include: cpuvar.h pmap.h specialreg.h
> >     src/sys/arch/x86/x86: bus_space.c cpu.c pmap.c
> >     src/sys/arch/xen/x86: cpu.c
> > Added Files:
> >     src/sys/arch/x86/include: pte.h
> >
> > Log Message:
> > Turn PMAP_NOCACHE into MI flag.
> > Add MI flags PMAP_WRITE_COMBINE, PMAP_WRITE_BACK, PMAP_NOCACHE_OVR.
> > Update pmap(9) manpage.
> >
> > hppa: Remove MD PMAP_NOCACHE flag as it exists as MI flag
> > mips: Rename MD PMAP_NOCACHE to PGC_NOCACHE.
> >
> > x86: Implement new MI flags using Page-Attribute Tables.
> > x86: Implement BUS_SPACE_MAP_PREFETCHABLE.
> >
> > Patch presented on tech-kern@:
> > http://mail-index.netbsd.org/tech-kern/2010/06/30/msg008458.html
> >
> > No comments on this last version.
>
> Some of us scarcely have had time to reply to the second version. :-/

It was enough time for code review. I thought, the way how it works
was clear after the discussion of the first version stopped.

> I don't think that a developer can predict from the manual page what
> effect the memory-type flags will have.  Some combination of the doco,
> API, and implementation needs improvement.

Please make your suggenstions.

> The x86 implementation gives the kernel more than one opportunity to
> program conflicting memory types on the same physical page.  The first
> opportunity to set conflicting types is an MI flag, PMAP_NOCACHE_OVR,
> that caters to a quirk of x86 where the "uncacheable" page attribute
> need not have "last say" on a page's memory type, but the MTRR can
> override.  The second opportunity to set conflicting types is by
> entering two mappings for the same page, VA1 -> PA and VA2 -> PA, VA1 !=
> VA2.

I don't see any conflicts.

PAT is per VA, MTRR is per PA.
The memory types are combined by the memory controller as described in
http://mail-index.netbsd.org/tech-kern/2010/05/20/msg008186.html
For more information, please find the APM Vol.2
http://support.amd.com/de/Processor_TechDocs/24593.pdf
chapter 7.8

You think, we use PAT since I committed my patch? That's wrong.
PAT is a hw feature which can't be disabled. Every x86 cpu showing
PAT in the cpuid feature list uses PAT.
My patch configures PAT to our needs (namely being able to specify
PMAP_WRITE_COMBINE) and gives MI a way to optimize access to
memory regions such as MMIO.

> Considering the potential conflicts, it seems that the memory-type flags
> are all "hints."  In that case, they should be described as such in the
> documentation, and there is no use for the hint PMAP_NOCACHE_OVR, which
> is identical to the PMAP_NOCACHE hint.

I think defining the memory characteristic is more than just a 'hint'.

> If the memory-type flags are not intended to be hints, but the
> implementation must obey the flags; or if sometimes the flags are hints,
> and sometimes they are mandatory; then either the implementation or the
> implementation and the API need improvement.

Please make your suggestions.

> The implementation needs to check for conflicts.  The API may need to
> accept both mandatory memory types and hints.  In this way, the behavior is
> predictable. 

Where do you see the conflicts?

Christoph