Re: [Xen-devel] RT Xen on ARM - R-Car series

[Julien Grall <julien.grall@xxxxxxx>]

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:
> > > On Thu, Jan 31, 2019 at 11:14:37PM +0000, Julien Grall wrote:
> > > > On 1/31/19 9:56 PM, Stefano Stabellini wrote:
> > > > > On Thu, 31 Jan 2019, Julien Grall wrote:
> > > > > > On 31/01/2019 12:00, Andrii Anisov wrote:
> > > > > > > On 31.01.19 13:37, Julien Grall wrote:
> > > So, I've got a hacky patch to 'fix' this on x86, by taking a reference
> > > to the mfn behind the virtual address provided when setting up the
> > > hypercall and mapping it in Xen.
> > That was the idea I had in mind :). Hopefully, no guest is modifying the
> > mapping (i.e the virtual address point to a different physical address)
> > afterwards.
> >
> > > This however doesn't work on ARM due
> > > to the lack of paging_gva_to_gfn. I guess there's something similar to
> > > translate a guest virtual address into a gfn or a mfn?
> > get_page_from_gva should to the job for you.
> > > +int map_runstate_area(struct vcpu *v,
> > > +                      struct vcpu_register_runstate_memory_area *area)
> > > +{
> > > +    unsigned long offset;
> > > +    unsigned int i;
> > > +    struct domain *d = v->domain;
> > > +    size_t size =
> > > +#ifdef CONFIG_COMPAT
> > > +        has_32bit_shinfo((v)->domain) ? sizeof(*v->compat_runstate_guest) :
> > > +#endif
> > > +                                        sizeof(*v->runstate_guest);
> > > +
> > > +    if ( v->runstate_guest )
> > > +    {
> > > +        ASSERT_UNREACHABLE();
> > > +        return -EBUSY;
> > > +    }
> > > +
> > > +    offset = area->addr.p & ~PAGE_MASK;
> > > +
> > > +    for ( i = 0; i < ARRAY_SIZE(v->runstate_mfn); i++ )
> > > +    {
> > > +        p2m_type_t t;
> > > +        uint32_t pfec = PFEC_page_present;
> > > +        gfn_t gfn = _gfn(paging_gva_to_gfn(v, area->addr.p, &pfec));
> > > +        struct page_info *pg;
> > > +
> > > +        if ( gfn_eq(gfn, INVALID_GFN) )
> > > +            return -EFAULT;
> > > +
> > > +        v->runstate_mfn[i] = get_gfn(d, gfn_x(gfn), &t);
> > get_gfn would need to be implemented on Arm.
> >
> > > +        if ( t != p2m_ram_rw || mfn_eq(v->runstate_mfn[i], INVALID_MFN) )
> > > +            return -EFAULT;
> > > +
> > > +        pg = mfn_to_page(v->runstate_mfn[i]);
> > > +        if ( !pg || !get_page_type(pg, PGT_writable_page) )
> > > +        {
> > > +            put_gfn(d, gfn_x(gfn));
> > > +            return -EFAULT;
> > > +        }
> > > +
> > > +        put_gfn(d, gfn_x(gfn));
> > > +
> > > +        if ( PFN_DOWN(gfn_x(gfn) + size) == PFN_DOWN(gfn_x(gfn)) )
> > This looks wrong, you seem to mix address and frame. I think you might want:
> >
> > if ( gfn_eq(gfn_add(gfn, PFN_DOWN(size)), gfn) )
> 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?
> 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.
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.
Cheers,

--
Julien Grall

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