Re: [PATCH 1/1] direct mmio for passthrough - kernel part

[Andrea Arcangeli <andrea@qumranet.com>]

On Wed, Apr 02, 2008 at 07:32:35AM +0300, Avi Kivity wrote:
> It ought to work.  gfn_to_hfn() (old gfn_to_page) will still need to take a 
> refcount if possible.

This reminds me, that mmu notifiers we could implement gfn_to_hfn only
with follow_page and skip the refcounting on the struct page.

I'm not suggesting that though, the refcount makes the code more
robust IMHO, and notably it allows to run on kernels without mmu
notifiers.

So the trick is to do something like

   if (pfn_valid(pfn))
      put_page(pfn_to_page(pfn));

That's the trick I was suggesting to take care of mmio space.

If the reserved ram is used instead of VT-d to allow DMA, that will
have change to:

   if (pfn_valid(pfn))
      page = pfn_to_page(pfn);
      if (page_count(page))
      	 put_page(page);

that will take care of both the reserved ram and the pfn.

In general I made it for the reserved-ram with only the page_count !=
0 check, because 'struct page' existed.

I'm unsure if it's good to add struct pages for non-ram, I find it
slightly confusing and not the right thing, it takes memory for stuff
that can't happen (refcounting only makes sense if the page finally
goes in the freelist when count reaches 0, and PG_dirty/referenced
bits and the like don't make sense either for non-ram or
reserved-ram). So I'm not sure the vmemmap trick is the best.

> This will increase the potential for type errors, so maybe we need to make 
> gfn_t and hfn_t distinct structures, and use accessors to get the actual 
> values.

That's also a possibility. If we go for this then hfn_t is probably a
better name as it's less likely to collide with core kernel VM
code. Otherwise perhaps "pfn" can be used instead of hfn, it's up to
you ;).

-------------------------------------------------------------------------
Check out the new SourceForge.net Marketplace.
It's the best place to buy or sell services for
just about anything Open Source.
http://ad.doubleclick.net/clk;164216239;13503038;w?http://sf.net/marketplace