Re: [Xen-devel] [PATCH] iommu: specify page_count rather than page_order to iommu_map/unmap()...

[Paul Durrant <Paul.Durrant@xxxxxxxxxx>]

> -----Original Message-----
> From: Jan Beulich [mailto:JBeulich@xxxxxxxx]
> Sent: 21 January 2019 12:05
> To: Paul Durrant <Paul.Durrant@xxxxxxxxxx>
> Cc: Julien Grall <julien.grall@xxxxxxx>; Andrew Cooper
> <Andrew.Cooper3@xxxxxxxxxx>; George Dunlap <George.Dunlap@xxxxxxxxxx>; Ian
> Jackson <Ian.Jackson@xxxxxxxxxx>; Roger Pau Monne <roger.pau@xxxxxxxxxx>;
> Wei Liu <wei.liu2@xxxxxxxxxx>; Sander Eikelenboom <linux@xxxxxxxxxxxxxx>;
> Chao Gao <chao.gao@xxxxxxxxx>; Jun Nakajima <jun.nakajima@xxxxxxxxx>;
> Kevin Tian <kevin.tian@xxxxxxxxx>; Stefano Stabellini
> <sstabellini@xxxxxxxxxx>; xen-devel <xen-devel@xxxxxxxxxxxxxxxxxxxx>;
> Konrad Rzeszutek Wilk <konrad.wilk@xxxxxxxxxx>; Tim (Xen.org)
> <tim@xxxxxxx>
> Subject: RE: [PATCH] iommu: specify page_count rather than page_order to
> iommu_map/unmap()...
> 
> >>> On 21.01.19 at 12:56, <Paul.Durrant@xxxxxxxxxx> wrote:
> >>  -----Original Message-----
> >> From: Jan Beulich [mailto:JBeulich@xxxxxxxx]
> >> Sent: 21 January 2019 11:28
> >> To: Paul Durrant <Paul.Durrant@xxxxxxxxxx>
> >> Cc: Julien Grall <julien.grall@xxxxxxx>; Andrew Cooper
> >> <Andrew.Cooper3@xxxxxxxxxx>; Roger Pau Monne <roger.pau@xxxxxxxxxx>;
> Wei
> >> Liu <wei.liu2@xxxxxxxxxx>; Sander Eikelenboom <linux@xxxxxxxxxxxxxx>;
> >> George Dunlap <George.Dunlap@xxxxxxxxxx>; Ian Jackson
> >> <Ian.Jackson@xxxxxxxxxx>; Chao Gao <chao.gao@xxxxxxxxx>; Jun Nakajima
> >> <jun.nakajima@xxxxxxxxx>; Kevin Tian <kevin.tian@xxxxxxxxx>; Stefano
> >> Stabellini <sstabellini@xxxxxxxxxx>; xen-devel <xen-
> >> devel@xxxxxxxxxxxxxxxxxxxx>; Konrad Rzeszutek Wilk
> >> <konrad.wilk@xxxxxxxxxx>; Tim (Xen.org) <tim@xxxxxxx>
> >> Subject: Re: [PATCH] iommu: specify page_count rather than page_order
> to
> >> iommu_map/unmap()...
> >>
> >> >>> On 18.01.19 at 17:03, <paul.durrant@xxxxxxxxxx> wrote:
> >> > ...and remove alignment assertions.
> >> >
> >> > Testing shows that certain callers of iommu_legacy_map/unmap()
> specify
> >> > order > 0 ranges that are not order aligned thus causing one of the
> >> > IS_ALIGNED() assertions to fire.
> >>
> >> As said before - without a much better explanation of why the current
> >> order-based model is unsuitable (so far I've been provided only vague
> >> pointers into "somewhere in PVH Dom0 boot code" iirc) to understand
> >> why it's undesirable to simply make those call sites obey to the
> current
> >> requirements, I'm not happy to see us go this route.
> >
> > I thought...
> >
> > "Using a count actually makes more sense because the valid
> > set of mapping orders is specific to the IOMMU implementation and to it
> > should be up to the implementation specific code to translate a mapping
> > count into an optimal set of mapping orders (when the code is finally
> > modified to support orders > 0)."
> >
> > ...was reasonably clear. Is that not a reasonable justification? What
> else
> > could I say?
> 
> Well, I was hoping to be pointed at the (apparently multiple) call sites
> where making them match the current function pattern is more involved
> and/or less desirable than changing the functions here.

AFAICT, one of them is memory.c:populate_physmap() where the extent order comes 
from the memop_args and the memory comes from alloc_domheap_pages(), which I 
don't believe aligns memory on the specified order. Regardless of the alignment 
though, the fact that order comes from a hypercall argument and may not match 
any of the orders supported by the IOMMU implementation makes me think that 
using a page count is better.

  Paul

> 
> Jan
> 


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