Re: [Xen-devel] [PATCH v5 07/10] xen/arm: Handle remove foreign mapping

[Ian Campbell <Ian.Campbell@xxxxxxxxxx>]

On Tue, 2013-12-17 at 15:06 +0000, Julien Grall wrote:
> On 12/17/2013 11:18 AM, Ian Campbell wrote:
> > On Mon, 2013-12-16 at 17:37 +0000, Julien Grall wrote:
> >> Modify get_page_from_gfn to take reference on foreign mapping. This will 
> >> avoid
> >> specific handling in the common code.
> >>
> >> Signed-off-by: Julien Grall <julien.grall@xxxxxxxxxx>
> >>
> >> ---
> >>     Changes in v4.2:
> >>         - get_page_from_gfn: move foreign checking before get_page
> >>         - add assert fdom != dom
> >>     Changes in v4.1:
> >>         - Remove specific p2m handling in common code
> >>         - Handle foreign mapping in get_page_from_gfn
> >>     Changes in v4:
> >>         - Split patch #6 from dom0 pvh series v6.2 to retrieve only common
> >>         code.
> >>         - Rework commit title
> >>         - Rename xen_rem_foreign_from_p2m in p2m_remove_foreign
> >>         - Get the mfn from the pte. We are not sure that maddr given in
> >>         parameters is valid
> >>     Changes in v3:
> >>         - Move put_page in create_p2m_entries
> >>         - Move xenmem_rem_foreign_from_p2m in arch/arm/p2m.c
> >>     Changes in v2:
> >>         - Introduce the patch
> >> ---
> >>  xen/arch/arm/p2m.c        |   15 +++++++++++++--
> >>  xen/include/asm-arm/p2m.h |   12 ++++++++++++
> >>  2 files changed, 25 insertions(+), 2 deletions(-)
> >>
> >> diff --git a/xen/arch/arm/p2m.c b/xen/arch/arm/p2m.c
> >> index 9bdcacd..d05fdff 100644
> >> --- a/xen/arch/arm/p2m.c
> >> +++ b/xen/arch/arm/p2m.c
> >> @@ -317,10 +317,21 @@ static int create_p2m_entries(struct domain *d,
> >>                  break;
> >>              case REMOVE:
> >>                  {
> >> -                    lpae_t pte;
> >> +                    lpae_t pte = third[third_table_offset(addr)];
> >> +                    unsigned long mfn;
> >> +
> >> +                    maddr = (pte.bits & PADDR_MASK & PAGE_MASK);
> >> +                    mfn = paddr_to_pfn(maddr);
> > 
> > FWIW mfn = pte.p2m.base, I think. I'm not sure we use this everywhere we
> > could.
> 
> The patch #8 which relinquish memory doesn't give the right mfn in
> parameters.
> It's because, you will need to look for each gfn the translated mfn (you
> can't assumed the mfn are contiguous). So the loop will go back to the
> version 3.

I'm not sure what you mean.

All I was trying to say is that:
        mfn = paddr_to_pfn(pte.bits & PADDR_MASK & PAGE_MASK)
is (probably) equivalent to:
        mfn = pte.p2m.base

> >> +
> >> +                    /* TODO: Handle other p2m type */
> >> +                    if ( pte.p2m.valid && p2m_is_foreign(pte.p2m.type) )
> > 
> > How useful is p2m_is_foreign now that this stuff doesn't need to be in
> > common code?
> 
> I'm not sure to understand. Do you mean the  macro?

Yes

> If so it's only for convenience.

Convenience is fair enoguh, especially once I noticed p2m_is_ram etc.

> 
> > Both of the above are really just observations rather than requests for
> > change, so:
> > Acked-by: Ian Campbell <ian.campbell@xxxxxxxxxx>
> 
> Thanks,
> 



_______________________________________________
Xen-devel mailing list
Xen-devel@xxxxxxxxxxxxx
http://lists.xen.org/xen-devel