>>> On 21.01.19 at 14:22, <paul.durr...@citrix.com> wrote: >> -----Original Message----- >> From: Jan Beulich [mailto:jbeul...@suse.com] >> Sent: 21 January 2019 12:05 >> To: Paul Durrant <paul.durr...@citrix.com> >> Cc: Julien Grall <julien.gr...@arm.com>; Andrew Cooper >> <andrew.coop...@citrix.com>; George Dunlap <george.dun...@citrix.com>; Ian >> Jackson <ian.jack...@citrix.com>; Roger Pau Monne <roger....@citrix.com>; >> Wei Liu <wei.l...@citrix.com>; Sander Eikelenboom <li...@eikelenboom.it>; >> Chao Gao <chao....@intel.com>; Jun Nakajima <jun.nakaj...@intel.com>; >> Kevin Tian <kevin.t...@intel.com>; Stefano Stabellini >> <sstabell...@kernel.org>; xen-devel <xen-devel@lists.xenproject.org>; >> Konrad Rzeszutek Wilk <konrad.w...@oracle.com>; Tim (Xen.org) >> <t...@xen.org> >> Subject: RE: [PATCH] iommu: specify page_count rather than page_order to >> iommu_map/unmap()... >> >> >>> On 21.01.19 at 12:56, <paul.durr...@citrix.com> wrote: >> >> -----Original Message----- >> >> From: Jan Beulich [mailto:jbeul...@suse.com] >> >> Sent: 21 January 2019 11:28 >> >> To: Paul Durrant <paul.durr...@citrix.com> >> >> Cc: Julien Grall <julien.gr...@arm.com>; Andrew Cooper >> >> <andrew.coop...@citrix.com>; Roger Pau Monne <roger....@citrix.com>; >> Wei >> >> Liu <wei.l...@citrix.com>; Sander Eikelenboom <li...@eikelenboom.it>; >> >> George Dunlap <george.dun...@citrix.com>; Ian Jackson >> >> <ian.jack...@citrix.com>; Chao Gao <chao....@intel.com>; Jun Nakajima >> >> <jun.nakaj...@intel.com>; Kevin Tian <kevin.t...@intel.com>; Stefano >> >> Stabellini <sstabell...@kernel.org>; xen-devel <xen- >> >> de...@lists.xenproject.org>; Konrad Rzeszutek Wilk >> >> <konrad.w...@oracle.com>; Tim (Xen.org) <t...@xen.org> >> >> Subject: Re: [PATCH] iommu: specify page_count rather than page_order >> to >> >> iommu_map/unmap()... >> >> >> >> >>> On 18.01.19 at 17:03, <paul.durr...@citrix.com> 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.
Of course it does (in MFN space). What I notice is that the gpfn passed in is not validated to be suitably aligned for the specified order. With guest_physmap_add_entry() processing each 4k page separately this doesn't currently have any bad effects, but I think it's a bug nevertheless. After all the comment in struct xen_memory_reservation's declaration says "size/alignment of each". The issue with fixing flaws like this is that there's always the risk of causing regressions with existing guests. > 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. Splitting up guest requests is orthogonal to whether a count or an order is more suitable as a parameter. Jan _______________________________________________ Xen-devel mailing list Xen-devel@lists.xenproject.org https://lists.xenproject.org/mailman/listinfo/xen-devel
