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

[George Dunlap <George.Dunlap@eu.citrix.com>]

On Wed, Jul 5, 2017 at 11:05 AM, Jan Beulich <JBeulich@suse.com> 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@suse.com>
> ---
> 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.

Everything else looks good, thanks.

 -George

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