Hi,
On Sat, Oct 30, 2010 at 06:55:42PM -0700, Chuck Silvers wrote:
On Wed, Oct 27, 2010 at 06:38:11PM +0900, Masao Uebayashi wrote:
On Tue, Oct 26, 2010 at 02:06:38PM -0700, Chuck Silvers wrote:
On Mon, Oct 25, 2010 at 02:09:43AM +0900, Masao Uebayashi wrote:
I think the uebayasi-xip branch is ready to be merged.
hi,
here's what I found looking at the current code in the branch:
- the biggest issue I had with the version that I reviewed earlier
was that it muddled the notion of a managed page. you wanted
to create a new kind of partially-managed page for XIP devices
which would not be managed by the UVM in the sense that it could
contain different things depending on what was needed but only that
the page's mappings would be tracked so that pmap_page_protect()
could be called on it. this is what led to all the pmap changes
the pmaps needed to be able to handle being called with a vm_page
pointer that didn't actually point to a struct vm_page.
it looks like you've gotten rid of that, which I like, but you've
replaced it with allocating a full vm_page structure for every page in
an XIP device, which seems like a waste of memory. as we discussed
earlier, I think it would be better to treat XIP pages as unmanaged
and change a few other parts of UVM to avoid needing to track the
mappings of XIP page mappings. I have thoughts on how to do all that,
which I'll list at the end of this mail. however, if XIP devices
are small enough that the memory overhead of treating device pages
as managed is reasonable, then I'll go along with it.
so how big do these XIP devices get?
It's waste of memory, yes. With 64M ROM on arm (4K page, 80byte vm_page),
the array is 1.25M. If vm_page's made a single pointer (sizeof(void
*) == 4), the array size becomes 64K. Not small difference.
Typical XIP'ed products would be mobile devices with FlashROM, or small
servers with memory disk (md or xmd). About 16M~1G RAM/ROM?
I made it back to have vm_page to simplify code. We can make it
to vm_page_md or whatever minimal, once after we figure out the
new design of MI vm_page_md.
either way, the changes to the various pmaps to handle the fake vm_page
pointers aren't necessary anymore, so all those changes should be
reverted.
Those mechanical vm_page - vm_page_md changes done in pmaps have
a valid point by itself. Passing around vm_page pointer across
pmap functions is unnecessary. I'd rather say wrong. All pmap
needs is vm_page_md.
I'd propose to do this vm_page - vm_page_md change in pmaps first
in HEAD and sync the branch with it, rather than revert it.
that seems a bit premature, don't you think?
since you're already talking about redesigning it?
... apparently not, you already checked it in.
It's not premature. It clarifies that passing struct vm_page * to
pmap is unnecessary at all. We'll need to move those MD PV data
structures to MI anyway.
This is what I'm having in my head:
struct vm_physseg {
:
paddr_t start, end;
:
struct vm_page *pgs;
:
struct vm_pv *pvs;
:
};
struct vm_pv {
/* == struct vm_page_md, or what's called PV head now */
};
struct vm_pv_entry {
/* == what's called PV entry now */
};
All pmaps are converted to use struct vm_pv array, instead of struct
vm_page::struct vm_page_md.
Here, the page identity is (struct vm_physseg *, off_t). You can
retrieve the physical address of a given struct vm_page * or struct
vm_pv *, by looking up the matching struct vm_physseg from the
global list (as talked about killing vm_page::phys_addr).
Of course XIP-capable, read-only physical segments have only vm_pv[].
I think tracking PV there for shared amap is worth doing in the
generic XIP design.
pmaps are passed struct vm_pv *, not struct vm_page *. Physical
address is looked up by calling a function.
it doesn't look like the PMAP_UNMANAGED flag that you added before
is necessary anymore either, is that right? if so, it should also
be reverted. if not, what's it for now?
pmap_enter() passes paddr_t directly to pmap. pmap has no clue if
the given paddr_t is to be cached/uncached. So a separate flag,
PMAP_UNMANAGED. Without it, a FlashROM which is registered as
(potentially) managed (using bus_space_physload_device) is always
mapped to user address as cached, even if it's not XIP. This made
userland flash writer program not work.
The point is whether to be cached or not is decided by virtual
address. Thus such an information should be stored in vm_map_entry
explicitly, in theory.
(This is related to framebuffers too.)
there is already an MI mechanism for
