Subject: Re: pmap tweak sanity check
To: None <Richard.Earnshaw@arm.com>
From: Chris Gilbert <chris@dokein.co.uk>
List: port-arm
Date: 11/12/2002 16:15:25
Richard Earnshaw said:
>> Index: pmap.c
>> ===================================================================
>> RCS file: /cvsroot/syssrc/sys/arch/arm/arm32/pmap.c,v
>> retrieving revision 1.121
>> diff -u -p -r1.121 pmap.c
>> --- pmap.c	2002/11/11 20:34:03	1.121
>> +++ pmap.c	2002/11/12 10:59:46
>> @@ -2303,7 +2303,7 @@ pmap_vac_me_user(struct pmap *pmap, stru
>>  			cpu_tlb_flushID();
>>  		}
>>  		cpu_cpwait();
>> -	} else if (entries > 0) {
>> +	} else if (entries != cacheable_entries) {
>>  		/*
>>  		 * Turn cacheing back on for some pages.  If it is a kernel
>>  		 * page, only do so if there are no other writable pages.
>>
>
>
> It might be clearer to write it as entries > cacheable_entries, but yes,
>  this should be OK.

True, anything that maintains clarity helps 8)

(sorry if this looks like random thoughts it kind of is, I'm just trying
to think of ways to make the pmap better, we're always going to have
problems due to having the virtual index, virtually tagged cache)

An optimisation I've just thought of, is it worth adding flags to the
pvh_attrs field of the vm_page_md stuff that indicates that all entries in
this vm_page list are readonly and therefore all cacheable?  that way when
we add a new readonly pv to the list we don't have to walk the list, we
just add it in.

Also add a flag that indicates if there is a writeable kernel entry, so
nothing is cacheable.  This cuts down the number of times we have to walk
the pv's list to do things, worst case will always be to walk it twice I
think.  However I'll admit I could be missing some other subtlety of the
pmap...

I think this means you end up with 6 cases for new additions:
Easy cases to handle, minimal work to do.
1. readonly kernel/user map to add. vm_page only has readonly entries, so
the new pv is cacheable.
2. readonly kernel/user map to add. there's a writeable kernel entry, so
must map it uncacheable.
3. readonly kernel map to add.  writeable user entry somewhere, make
mapping uncached.

Potentially have to walk the whole list.
4. readonly user map to add.  writeable user entry somewhere (all readonly
flag not set, kernel writeable flag not set). Walk the list, looking for a
mapping in the same pmap, if there is one, use it's cache setting, if
there isn't we're safe to be cacheable, as the writeable entry is in
another pmap.

Will always have to walk the whole list.
5. writeable kernel mapping, if there isn't already a writeable kernel
mapping, all mappings must now be uncached, update the flags and de-cache
everything.
6. writeable user mapping, update the flags, make the kernel and that pmap
mappings uncached. (can exit early if we find they're already uncached due
to another writeable mapping)

For removing/changing a pv entry in a list, I think the cases are:
7. Changing from readonly->writeable entries will have the same effect as
cases for 5 and 6.  IE potentially a full list walk.

8. Changing from writeable->readonly will need full list walk to see if
there are any other writeable entries in the same pmap (hmmm, perhaps add
a new field in the md part with the count of writeable mappings?)  Then
another full list walk to reenable caching.

9. Removing a readonly mapping will have no effect on cacheability.

10. Removing a writeable mapping will have a similair effect to changing
from writeable->readonly (see 8.)

The other thing to consider is a writeable mapping with no other mappings
in the same pmap/kernel should be cached, and uncached if another mapping
is added...

Am I missing any other things to consider?

Chris