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

["Jan Beulich" <JBeulich@xxxxxxxx>]

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

> Going through the code, both of the shadow path and (post-modification)
> pv mm path won't call get_page_from_l1e with non-present l1e AFAICT.

Correct, or else the assertion would be wrong. I'm putting it there
just to make sure new violations of the assumption won't be
introduced.

Jan



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