Re: [Xen-devel] IOREQ server on Arm

[Paul Durrant <Paul.Durrant@xxxxxxxxxx>]

> -----Original Message-----
> From: Julien Grall [mailto:julien.grall@xxxxxxx]
> Sent: 27 September 2018 11:31
> 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/27/2018 11:16 AM, Paul Durrant wrote:
> >> -----Original Message-----
> >> From: Julien Grall [mailto:julien.grall@xxxxxxx]
> >> Sent: 27 September 2018 10:42
> >> 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/27/2018 09:38 AM, Paul Durrant wrote:
> >>>> -----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.
> >>
> >> I understand that you don't have reference count in the P2M (that's the
> >> same on Arm today except for foreign mapping). But I think I can list
> at
> >> least 2 major issues with the design today. Let me give an example
> based
> >> on my understanding.
> >>
> >>     1. DM requests to map the IOREQ page
> >>    a) page allocated (one reference)
> >>    b) get reference (will be dropped when the IOREQ server is
> >> destroyed)
> >>
> >>     2. DM requests to map the IOREQ page (second time)
> >>    No reference taken
> >>
> >>     3. DM unmap the IOREQ page
> >>     4. DM unmap the IOREQ page
> >>
> >> AFAIU, 3. 4. would be done through XENMEM_remove_from_physmap. So no
> >> reference dropped there. While the reference 1.b) will be dropped in
> >> hvm_free_ioreq_mfn. AFAICT 1.a) would be kept until the domain die.
> This
> >> would result to Xen memory exhaustion in long term. Did I miss
> anything?
> >>
> >
> > 1.a) would be kept until the IOREQ server is destroyed, which will
> happen either at domain destruction *or* when the DM destroys it.
> 
> Argh, I mixed get_page_type and get_page(). Sorry for the noise.
> 
> >
> >> But, I think there are another way for badly written guest to remove
> the
> >> page. It looks like you can use XENMEM_decrease_reservation as the page
> >> belongs to the guest. So a reference would be dropped by 3. and 4.
> >>
> >
> > How so? The pages don't belong to the guest; they belong the the DM
> domain. They never appear in guest P2M so how can the guest
> decrease_reservation them? Are you worried about the DM domain doing a
> decrease reservation?
> 
> By guest I meant DM domain.
> 

If the DM domain is not PV then currently it must be the hardware domain to be 
able to map resources. Hence we trust it not to descrease_reservation IOREQ 
pages.

  Paul

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