Re: [PATCH for-4.12 1/8] dom0/pvh: align allocation and mapping order to start address

["Jan Beulich" <JBeulich@suse.com>]

>>> On 05.02.19 at 11:54, <andrew.cooper3@citrix.com> wrote:
> On 05/02/2019 07:42, Jan Beulich wrote:
>>>>> On 04.02.19 at 18:11, <roger.pau@citrix.com> wrote:
>>> On Mon, Feb 04, 2019 at 09:41:34AM -0700, Jan Beulich wrote:
>>>>>>> On 30.01.19 at 11:36, <roger.pau@citrix.com> wrote:
>>>>> Due to the recent changes in the iommu mapping logic, the start
>>>>> addresses provided need to be aligned to the order intended to be
>>>>> mapped.
>>>> Irrespective of your reply to Wei's similar request (where you've
>>>> provided links to mails showing crashes) I'd like you to explain
>>>> this better. This is in particular because I don't really see what
>>>> "recent changes in the iommu mapping logic" you talk about.
>>> Commit 725bf00a87f ("iommu / p2m: add a page_order parameter to
>>> iommu_map/unmap_page()...") added the following two asserts to
>>> iommu_map:
>>>
>>> ASSERT(IS_ALIGNED(dfn_x(dfn), (1ul << page_order)));
>>> ASSERT(IS_ALIGNED(mfn_x(mfn), (1ul << page_order)));
>>>
>>> Previously iommu_map would add unaligned entries without complaining,
>>> but now in debug builds the assert will trigger.
>> Right, but the assertions were added to ensure expected behavior,
>> not to change anything.
> 
> No - this isn't reasonable.
> 
> Those assertions were added "because noone should be violating them".
> 
> As it turns out, that expectation was false.  There are real codepaths
> which do trip this assert, which functioned correctly before.
> 
> The two options are to either to bugfix the assertion failures by
> removing the assertions, or do some code improvement to update callers
> to be consistent with the new, different, expectation.
> 
> Roger was correct to being with.  The IOMMU code has recently shifted
> expectations, in a way which currently malfunctions only in debug builds.

I'm sorry, but no, I can't agree with such a position: Be it P2M or
IOMMU, requesting larger order mappings with frame numbers not
suitably aligned for the requested order is a mistake. It is the very
nature of "order" passed to both allocation and mapping functions
that returned blocks or memory or produced mappings adhere to
the requested order in terms of both size and alignment. (Was it
you or someone else who recently asked whether an allocation
with order > 0 would produce an order-aligned chunk of memory?)
For anything else it should indeed be a count rather than an order
to be passed in.

It's a second mistake for any code to have accepted such inputs.

Jan



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