Re: XIP
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
Re: XIP
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 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 specifying cachability of mappings, see PMAP_NOCACHE and related flags. > > - I mentioned before that I think the driver for testing this using > >normal memory to simulate an XIP device should just be the existing > >"md" driver, rather than a whole new driver whose only purpose > >would be testing the XIP code. however, you wrote a separate > >"xmd" driver anyway. so I'll say again: I think the "xmd" should be > >merged into the "md" driver. > > It has turned out that the new xmd(4) is not really md(4); it's not > md(4) modified to support XIP > but rather > rom(4) emulated using RAM > > I plan to merge xmd(4) into rom(4) or flash(4) when it's ready. > If you don't like xmd(4) to reserve dev namespace, I'd rather back > out xmd(4) totally. > > If you don't like the name, suggest a better one. ;) I'll respond to this in the other thread you started about it. but creating a temporary device driver does seem pretty silly. > >you also have an "xmd_machdep.c" for various platforms, but nothing > >in that file is really machine-specific. rather than use the > >machine-specific macros for converting between bytes and pages, > >it would be better to either use the MI macros (ptoa / atop) > >or jus
Re: Fixes for kern/40018 -- any chance of getting these pulled into the -current and 5.x trees?
On Fri, Oct 29, 2010 at 04:32:28PM -0700, Brian Buhrow wrote: > Hello. Is this something you can reproduce easily? I wonder if the > ethernet chip is just hanging, and the rest of the machine is fine? The machine was fine otherwise. I didn't have time last night to experiment, will reproduce and narrow it down, but this will take a few days... I'm not yet sure what details are important to reproduce it, I was surised to hit this state after a few weeks w/o problems - but let me double check I used the patched kernel at all before we continue to worry. Martin
