Re: [Xen-devel] [PATCH V7 3/5] x86/altp2m: fix display frozen when switching to a new view early

[George Dunlap <George.Dunlap@xxxxxxxxxx>]

> On Nov 20, 2018, at 9:12 AM, Jan Beulich <JBeulich@xxxxxxxx> wrote:
> 
>>>> On 19.11.18 at 18:26, <rcojocaru@xxxxxxxxxxxxxxx> wrote:
>> When an new altp2m view is created very early on guest boot, the
>> display will freeze (although the guest will run normally). This
>> may also happen on resizing the display. The reason is the way
>> Xen currently (mis)handles logdirty VGA: it intentionally
>> misconfigures VGA pages so that they will fault.
>> 
>> The problem is that it only does this in the host p2m. Once we
>> switch to a new altp2m, the misconfigured entries will no longer
>> fault, so the display will not be updated.
>> 
>> This patch:
>> * updates ept_handle_misconfig() to use the active altp2m instead
>>  of the hostp2m;
>> * modifies p2m_change_entry_type_global(),
>>  p2m_memory_type_changed(), p2m_change_type_range() and
>>  p2m_finish_type_change() to propagate their changes to all
>>  valid altp2ms.
>> 
>> With the introduction of altp2m fields in p2m_memory_type_changed()
>> the whole function has been put under CONFIG_HVM.
>> 
>> Signed-off-by: Razvan Cojocaru <rcojocaru@xxxxxxxxxxxxxxx>
>> Suggested-by: George Dunlap <george.dunlap@xxxxxxxxxx>
> 
> Judging from George's earlier analysis I wonder whether the patch
> ordering is correct: I've got the impression that the patch here should
> be last in the series, for it to be correct and efficient in all cases.

My patches back would require significant rework — both of my patches to rebase 
on an earlier tree, and of Razvan’s patches to be rebased later.  I don’t think 
this kind of thing should be required unless there is a compelling benefit to 
doing so.

Normally the reason for such an ordering is “no regressions in the middle of 
the series”, primarily in order to avoid breaking bisection; and of course 
there’s also something  much more aesthetically satisfying about doing a bunch 
of prep work behind the scenes and then flipping one switch to enable it at the 
end of the series.

In this case, altp2m + logdirty was already broken; so I didn’t think this 
patch could be considered to introduce a regression.  Thus the only reason to 
have this patch be the final patch would be for aesthetic purposes, which I 
didn’t consider enough value to justify requesting a patch re-ordering.

Did you have a compelling reason in mind for doing the reordering?

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