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.

Oh, I think this is buggy in another way than I first thought: It
looks like it's meant to inherit the original flags (as they're not
being cleared from "flags"), but afaict that breaks the next
level field - that one is also being inherited, and then being
OR-ed into with the new next level field. I suppose all of this
has not caused issues simply because we've not allowed
shared page tables on AMD for quite a while (and both fields
are of no interest in the shadow mode case).

I'll drop the use of p2m_add_iommu_flags() in that particular
case altogether.

Jan


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