Re: [Xen-devel] [PATCH 1/5] x86/traps: Drop paging_mode_external() handling from the PV pagefault path

[Andrew Cooper <andrew.cooper3@xxxxxxxxxx>]

On 12/12/16 11:27, Tim Deegan wrote:
> At 10:43 +0000 on 12 Dec (1481539421), Andrew Cooper wrote:
>> PV guests necessarily can't be external, as Xen must steal address space from
>> them.  Pagefaults for HVM guests are handled by {vmx,svm}_vmexit_handler() 
>> and
>> don't enter the PV fixup_page_fault() path.
>>
>> This paging_fault() callsite is therefore dead code, so drop it.
>>
>> Clarify the comment at the other paging_fault() callsite to indicate that it
>> covers the logdirty case only.
>>
>> No functional change.
> IMO this is a change, just not on any supported config.

Really?  I'd agree if the content of the clause was reachable (and we
didn't support the configuration required to make it reachable), but my
argument here is that it is genuinely dead code and cannot be reached.

The only way to make paging_mode_external(d) true would cause Xen to be
using a different path to service the pagefault.

This particular piece of code makes me wonder whether HVM guests used to
use the PV pagefault path, but if they ever did, they don't any more.

>
>> -    /* For non-external shadowed guests, we fix up both their own 
>> -     * pagefaults and Xen's, since they share the pagetables. */
>> +    /*
>> +     * For non-external shadowed guests (i.e. PV guests with logdirty
>> +     * active), we fix up both their own pagefaults and Xen's, since
>> +     * they share the pagetables.
>> +     */
>>      if ( paging_mode_enabled(d) && !paging_mode_external(d) )
> Here we can drop the check of !paging_mode_external(d), or maybe turn
> it into an assertion somewhere.
>
> With that,
>
> Acked-by: Tim Deegan <tim@xxxxxxx>

Seeing as both you and Jan have asked, I will see about addition an
assertion.  However, it will have to be in patch 3 for bisectability,
when we rule out the use of PV refcount guests.  Are you ok with that?

~Andrew

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