Re: [Xen-devel] [PATCH 2/2] iommu: elide flushing for higher order map/unmap operations

[Paul Durrant <Paul.Durrant@xxxxxxxxxx>]

> -----Original Message-----
> From: Jan Beulich [mailto:JBeulich@xxxxxxxx]
> Sent: 03 December 2018 10:21
> To: Andrew Cooper <Andrew.Cooper3@xxxxxxxxxx>; Paul Durrant
> <Paul.Durrant@xxxxxxxxxx>
> Cc: Brian Woods <brian.woods@xxxxxxx>; Suravee Suthikulpanit
> <suravee.suthikulpanit@xxxxxxx>; Julien Grall <julien.grall@xxxxxxx>;
> Roger Pau Monne <roger.pau@xxxxxxxxxx>; Wei Liu <wei.liu2@xxxxxxxxxx>;
> Kevin Tian <kevin.tian@xxxxxxxxx>; Stefano Stabellini
> <sstabellini@xxxxxxxxxx>; xen-devel <xen-devel@xxxxxxxxxxxxxxxxxxxx>
> Subject: Re: [PATCH 2/2] iommu: elide flushing for higher order map/unmap
> operations
> 
> >>> On 30.11.18 at 13:49, <andrew.cooper3@xxxxxxxxxx> wrote:
> > On 30/11/2018 10:45, Paul Durrant wrote:
> >> +enum iommu_flush_type
> >> +{
> >> +    IOMMU_FLUSH_none, /* no flush required */
> >> +    IOMMU_FLUSH_added, /* no modified entries, just additional entries
> */
> >
> > IOMMU_FLUSH_invalid ?  I think it is more descriptive of the scenario in
> > which it is used.
> 
> This reads in a pretty misleading way to me. IOMMU_FLUSH_new
> or IOMMU_FLUSH_new_ents perhaps? Otoh I was quite okay with
> the name Paul had chosen.

I'll stick with those then :-)

> 
> >> @@ -177,7 +184,8 @@ struct iommu_ops {
> >>       * other by the caller in order to have meaningful results.
> >>       */
> >>      int __must_check (*map_page)(struct domain *d, dfn_t dfn, mfn_t
> mfn,
> >> -                                 unsigned int flags);
> >> +                                 unsigned int flags,
> >> +                                 enum iommu_flush_type *flush_type);
> >
> > Maintaining the flush type by pointer is quite awkward.
> >
> > How about folding a positive flush type in with negative errors?  i.e.
> > map_page() becomes < 0 on error, 0 for success/no flush and >0 for
> > success/w flush.
> >
> > I think the result would be rather cleaner to read.
> 
> It would, but mapping operations with higher orders may fail
> _and_ require flushing.
> 

Agreed. I did not want to confuse the error semantics with the flush semantics. 
There is no order at this level as yet but, when one is added, this separation 
- as you say - will become important.

  Paul

> Jan
> 


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