[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

Re: [Xen-devel] IOREQ server on Arm



> -----Original Message-----
> From: Julien Grall [mailto:julien.grall@xxxxxxx]
> Sent: 26 September 2018 22:32
> To: Paul Durrant <Paul.Durrant@xxxxxxxxxx>; 'Jan Beulich'
> <JBeulich@xxxxxxxx>
> Cc: 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
> 
> 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?

Well, as Jan says, on x86 we don't reference count in the P2M so multiple 
mappings should not be an issue AFAICT. It sounds like resource mapping should 
be treated the same way as foreign mapping (albeit with a non-foreign domid) 
such that the reference acquisition occurs at map time.

  Paul

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

 


Rackspace

Lists.xenproject.org is hosted with RackSpace, monitoring our
servers 24x7x365 and backed by RackSpace's Fanatical Support®.