Re: RT Xen on ARM - R-Car series

["Roger Pau Monné" <roger.pau@citrix.com>]

On Mon, Feb 04, 2019 at 09:58:43PM +0000, Julien Grall wrote:
> Hi,
> 
> On 2/4/19 12:53 PM, Roger Pau Monné wrote:
> > On Fri, Feb 01, 2019 at 05:40:14PM +0000, Julien Grall wrote:
> > > Hi,
> > > 
> > > On 01/02/2019 16:53, Roger Pau Monné wrote:
> > Thanks!
> > 
> > Here is an updated version which seems to build on ARM. I don't have
> > an easy way to test this, could you give it a spin?
> 
> Thank you for writing a patch. I am back in France this week for family
> reason and will not have time properly give a spin this week. Stefano,
> Andrii, can you test it?

No problem. I'm also happy if someone wants to pick this up, test it
and formally post it to xen-devel as a patch with whatever changes are
required.

> > 
> > I don't like adding CONFIG_X86/ARM in common code, so it might be
> > worth to either try to factor this out into arch specific code, or
> > even better, provide common functions to translate a guest virtual
> > address into a gfn, mfn or page.
> 
> I would prefer GVA to MFN/page because we already have a fairly complicate
> helper handling all the cases (e.g memaccess, break-before-make, ...) for
> translation and taking a reference on the page. Although we could
> potentially split the function in two if we want to cater any other
> translation.

I think providing a helper to translate a guest VA into a gfn might be
helpful in some cases, but I don't have a strong opinion. I would
however prefer to have some kind of common helper here that's
implemented for all arches.

> One comment below.
> 
> > +#elif defined(CONFIG_ARM)
> > +        pg = get_page_from_gva(v, area->addr.p, GV2M_WRITE);
> > +        if ( !pg || !get_page_type(pg, PGT_writable_page) )
> 
> One reference is already taken by get_page_from_gva. So you should not need
> to take another here.

This is taking a type reference, which is a noop on ARM, but matters
on x86. This needs to be done on ARM so that the cleanup path can be
shared between x86 and ARM, which uses put_page_and_type.

get_page_from_gva performs a get_page, but not a get_page_type (or a
get_page_and_type).

Thanks, Roger.

_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xenproject.org
https://lists.xenproject.org/mailman/listinfo/xen-devel