Re: [Xen-devel] [PATCH] x86/mm: re-arrange get_page_from_l<N>e() vs pv_l1tf_check_l<N>e

[Wei Liu <wei.liu2@xxxxxxxxxx>]

On Mon, Aug 20, 2018 at 05:37:32AM -0600, Jan Beulich wrote:
> >>> On 20.08.18 at 11:59, <wei.liu2@xxxxxxxxxx> wrote:
> > On Fri, Aug 17, 2018 at 12:42:31AM -0600, Jan Beulich wrote:
> >> Restore symmetry between get_page_from_l<N>e(): pv_l1tf_check_l<N>e is
> >> uniformly invoked from outside of them. They're no longer getting called
> >> for non-present PTEs. This way the slightly odd three-way return value
> >> meaning of the higher level ones can also be got rid of.
> >> 
> >> Introduce local variables holding the page table entries processed, and
> >> use them throughout the loop bodies instead of re-reading them from the
> >> page table several times.
> >> 
> >> Signed-off-by: Jan Beulich <jbeulich@xxxxxxxx>
> >> 
> >> --- a/xen/arch/x86/mm.c
> >> +++ b/xen/arch/x86/mm.c
> >> @@ -900,8 +900,11 @@ get_page_from_l1e(
> >>      struct domain *real_pg_owner;
> >>      bool write;
> >>  
> >> -    if ( !(l1f & _PAGE_PRESENT) )
> >> +    if ( unlikely(!(l1f & _PAGE_PRESENT)) )
> >> +    {
> >> +        ASSERT_UNREACHABLE();
> >>          return 0;
> >> +    }
> > 
> > Why is this needed here? According to commit message get_page_from_l1e
> > shouldn't be called with non-present l1e.
> 
> Correct, hence the assertion. Othe than its higher-level siblings,
> this function is non-static, and hence I felt it warranted to have
> such an assertion.

With the above put into commit message:

Reviewed-by: Wei Liu <wei.liu2@xxxxxxxxxx>

_______________________________________________
Xen-devel mailing list
Xen-devel@xxxxxxxxxxxxxxxxxxxx
https://lists.xenproject.org/mailman/listinfo/xen-devel