Re: [Xen-devel] [PATCH] dump_p2m_table: For IOMMU

["Jan Beulich" <JBeulich@xxxxxxxx>]

>>> On 10.08.12 at 15:41, Wei Wang <wei.wang2@xxxxxxx> wrote:
> On 08/10/2012 02:52 PM, Jan Beulich wrote:
>>>>> On 10.08.12 at 12:50, Wei Wang<wei.wang2@xxxxxxx>  wrote:
>>> On 08/09/2012 09:26 AM, Jan Beulich wrote:
>>>> (And similar
>>>> to the issue Santosh has already fixed here - the original
>>>> function pointlessly maps/unmaps the page when "level<= 1".
>>>> Furthermore, iommu_map.c has nice helper functions
>>>> iommu_next_level() and amd_iommu_is_pte_present() - why
>>>> aren't they in a header instead, so they could be used here,
>>>> avoiding the open coding of them?)
>>>
>>> Maybe those helps appears after the original function. I could sent a
>>> patch to clean up these:
>>> * do not map/unmap if level<= 1
>>> * move amd_iommu_is_pte_present() and iommu_next_level() to a header
>>> file. and use them in deallocate_next_page_table.
>>> * Using next_level instead of recalculation (if requested)
>>
>> Yes, please. As to using next_level - it depends, besides the above,
>>   on how bad it is if this is really wrong; an ASSERT() or BUG_ON()
>> might be on order here.
> 
> How about ASSERT(next_level < = (level -1) )?

But that would again mean levels can be skipped. If they can,
then using next_level is the way to go, and the assertion is fine.
If they can't, the assertion should say ==.

Jan


_______________________________________________
Xen-devel mailing list
Xen-devel@xxxxxxxxxxxxx
http://lists.xen.org/xen-devel