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

[Tim Deegan <tim@xxxxxxx>]

At 11:45 +0000 on 12 Dec (1481543145), Andrew Cooper wrote:
> 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.

Ah, you're right - I was thinking of refcounts mode.  External mode
guests can't get here.  In which case...

> >> +    /*
> >> +     * 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?

...I don't follow you here -- since external-mode guests can't reach this
code we can surely switch from test to assert.

Tim.

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