Re: [Xen-devel] [PATCH v2 1/3] x86/p2m-pt: simplify p2m_next_level()

["Jan Beulich" <JBeulich@xxxxxxxx>]

>>> On 27.07.17 at 18:19, <George.Dunlap@xxxxxxxxxxxxx> wrote:
> On Wed, Jul 5, 2017 at 11:05 AM, Jan Beulich <JBeulich@xxxxxxxx> wrote:
>> Calculate entry PFN and flags just once. Convert the two successive
>> main if()-s to and if/elf-if chain. Restrict variable scope where
>> reasonable. Take the opportunity and also make the induction variable
>> unsigned.
>>
>> Signed-off-by: Jan Beulich <jbeulich@xxxxxxxx>
>> ---
>> v2: Re-do mostly from scratch following review feedback.
>> Note: I have trouble seeing how the old code worked, when the 2M page
>>       shattering path specified neither read nor write permission for
>>       the IOMMU. Am I overlooking a reason why this was (and should
>>       remain) so?
> 
> Hmm -- given that in all other circumstances, a 4k page which is
> ram_rw gets RW, then I think the old code must certainly be buggy.
> 
> But is your code correct?  It looks like you unconditionally give it
> RW, when for ram_ro, for example it should be R (not W).  It seems
> like we should either call p2m_get_iommu_flags(), or ASSERT() that the
> resulting flags would be RW.

Hmm, good point, but that means the original code had the same
issue when splitting 1G into 2M pages. I'd prefer to not call
p2m_get_iommu_flags() though, but instead simply inherit the
IOMMU flags from the original large page.

Jan


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