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

["Jan Beulich" <JBeulich@suse.com>]

>>> On 20.11.18 at 10:43, <George.Dunlap@citrix.com> wrote:

> 
>> On Nov 20, 2018, at 9:12 AM, Jan Beulich <JBeulich@suse.com> wrote:
>> 
>>>>> On 19.11.18 at 18:26, <rcojocaru@bitdefender.com> 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@bitdefender.com>
>>> Suggested-by: George Dunlap <george.dunlap@citrix.com>
>> 
>> 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?

No, it merely looked wrong to me from earlier discussion. If staying
with the current order is fine with you, it'll be fine with me as well.
(It wasn't clear to me that re-ordering would be significant effort.)

Jan


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