Re: [Xen-devel] IOREQ server on Arm

[Julien Grall <julien.grall@xxxxxxx>]

Hi Paul,

On 09/26/2018 01:01 PM, Paul Durrant wrote:
> > -----Original Message-----
> > From: Jan Beulich [mailto:JBeulich@xxxxxxxx]
> > Sent: 26 September 2018 12:57
> > To: Paul Durrant <Paul.Durrant@xxxxxxxxxx>
> > Cc: Julien Grall <julien.grall@xxxxxxx>; Andrew Cooper
> > <Andrew.Cooper3@xxxxxxxxxx>; Roger Pau Monne <roger.pau@xxxxxxxxxx>;
> > Stefano Stabellini <sstabellini@xxxxxxxxxx>; xen-devel <xen-
> > devel@xxxxxxxxxxxxxxxxxxxx>
> > Subject: RE: IOREQ server on Arm
> >
> > > > > On 26.09.18 at 13:02, <Paul.Durrant@xxxxxxxxxx> wrote:
> > > --- a/xen/common/memory.c
> > > +++ b/xen/common/memory.c
> > > @@ -1105,8 +1105,11 @@ static int acquire_resource(
> > >
> > >           for ( i = 0; !rc && i < xmar.nr_frames; i++ )
> > >           {
> > > -            rc = set_foreign_p2m_entry(currd, gfn_list[i],
> > > -                                       _mfn(mfn_list[i]));
> > > +            rc = (xmar.flags & XENMEM_rsrc_acq_caller_owned) ?
> > > +                guest_physmap_add_entry(currd, gfn_list[i],
> > > +                                        _mfn(mfn_list[i]), 0,
> > p2m_ram_rw) :
> > > +                set_foreign_p2m_entry(currd, gfn_list[i],
> > > +                                      _mfn(mfn_list[i]));
> > >               /* rc should be -EIO for any iteration other than the first
> > */
> > >               if ( rc && i )
> > >                   rc = -EIO;
> > >
> > > But the guest_physmap_add_entry() is problematic as it will IOMMU map
> > pages
> > > as well, which is probably not wanted.
> > Yeah, I'd prefer if we avoided establishing IOMMU mappings here.
> > How about transforming set_foreign_p2m_entry() into
> > set_special_p2m_entry(), with a type passed in?
> That sounds like it might work.
>
> Julien, do you want page types to distinguish caller-owned resources from 
> normal RAM are you ok with p2m_ram_rw even though it could be subject of 
> another domain's foreign map?
Based on your previous e-mail, I would be fine with that on Arm.

This brings me to the next question. Do you expect set_special_p2m_entry 
to take a reference on the page?
If not, we may run into some troubles because AFAICT you can map twice 
the ioreq page in a guest but reference will only be taken on the 
allocation.
However, the unmap path will always drop a reference when removing the 
page. This is because Xen at the moment, reference will not be taken on 
mapping but allocation (we assume a page could not be mapped twice in a 
guest).
Foreign mapping on Arm are a bit special because we get a reference on 
mapping them and will drop it when the mapping disappear. So we would 
not have any problem there.
Any thoughts?

--
Julien Grall

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