Wow, gfn_to_mfn() really is a no-op for the pv case. I thought
you meant that it didn't do any translation, but it appears it
also doesn't even set the return value to INVALID_MFN, so for pv
I have to add a separate check for mfn_valid() to see if it
is even a valid Xen mfn! THAT was what was causing the crash.
Also, it appears that gfn_to_mfn_unshare is not suitable for
tmem... it appears the existing code checks for "must_succeed"
but gives no indication of success if must_succeed is NOT set
but mem_sharing_unshare_page fails, correct? If I do plan to
write to the page but I am willing to accept failure if the
page is shared (since tmem is resilient to this scenario),
it appears I will have to add a new variation of gfn_to_mfn_unshare.
Or maybe I should just overload must_succeed, and if must_succeed
is 2 and unsharing fails, return INVALID_MFN?
P.S. Here's what the relevant tmem code looks like now (below
and attached). Quick review appreciated.
static inline void *cli_get_page(tmem_cli_mfn_t cmfn, unsigned long *pcli_mfn,
pfp_t **pcli_pfp, bool_t cli_write)
{
unsigned long xmfn;
p2m_type_t t;
struct page_info *page;
int ret;
if ( is_hvm_domain(current->domain) )
{
xmfn = mfn_x(gfn_to_mfn_unshare(current->domain, cmfn, &t, 2));
if (t != p2m_ram_rw || xmfn == INVALID_MFN)
return NULL;
}
else
{
xmfn = cmfn;
if (!mfn_valid(xmfn))
return NULL;
}
page = mfn_to_page(xmfn);
if ( cli_write )
ret = get_page_and_type(page, current->domain, PGT_writable_page);
else
ret = get_page(page, current->domain);
if ( ret <= 0 )
return NULL;
*pcli_mfn = xmfn;
*pcli_pfp = (pfp_t *)page;
return map_domain_page(xmfn);
}
static inline void cli_put_page(void *cli_va, pfp_t *cli_pfp,
unsigned long cli_mfn, bool_t mark_dirty)
{
if ( mark_dirty )
paging_mark_dirty(current->domain,cli_mfn);
put_page((struct page_info *)cli_pfp);
unmap_domain_page(cli_va);
}
> -----Original Message-----
> From: Dan Magenheimer
> Sent: Monday, September 20, 2010 8:13 AM
> To: Keir Fraser; Jan Beulich
> Cc: Xen-devel
> Subject: RE: [Xen-devel] Re: xen crash in tmem: checking a xen pfn for
> domain ownership
>
> > On 20/09/2010 08:16, "Jan Beulich" <JBeulich@xxxxxxxxxx> wrote:
> >
> > >>>> On 17.09.10 at 21:32, Keir Fraser <keir.fraser@xxxxxxxxxxxxx>
> > wrote:
> > >> Oh, depending on what you want to do with the page you may well
> want
> > to
> > >> get_page(current->domain, page). You don't hold a lock on the
> > domain's p2m,
> > >> so page ownerships can change under your feet, and hence getting a
> > reference
> > >> to the page, and checking the page's ownership at the same time,
> > might be
> > >> wise. And if you want to modify the page you should probably use
> > >> get_page_and_type(..., PGT_writable_page).
> > >
> > > That's particularly important for the pv case, where gfn_to_mfn()
> is
> > > a no-op.
>
> Thanks, Jan and Keir, I'll take a look at it.
>
> > Yes, I actually forgot about the PV case, and I think that's the only
> > case
> > that matters for tmem. :-)
>
> Tmem works for HVM guests now too (on top of Stefano's PV-on-HVM
> Linux kernel patches), though I haven't tested it in awhile.
tmem_xen.c
Description: Binary data
_______________________________________________
Xen-devel mailing list
Xen-devel@xxxxxxxxxxxxxxxxxxx
http://lists.xensource.com/xen-devel
|
Home