[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

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

>>> 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.


Xen-devel mailing list



Lists.xenproject.org is hosted with RackSpace, monitoring our
servers 24x7x365 and backed by RackSpace's Fanatical Support®.