Re: [PATCH v15 1/3] mem_sharing: don't reset vCPU info page during fork reset

[Tamas K Lengyel <tamas@xxxxxxxxxxxxx>]

On Mon, Apr 20, 2020 at 9:51 AM Jan Beulich <jbeulich@xxxxxxxx> wrote:
>
> On 20.04.2020 16:27, Tamas K Lengyel wrote:
> > On Mon, Apr 20, 2020 at 8:19 AM Jan Beulich <jbeulich@xxxxxxxx> wrote:
> >>
> >> On 20.04.2020 16:15, Tamas K Lengyel wrote:
> >>> On Mon, Apr 20, 2020 at 1:45 AM Roger Pau Monné <roger.pau@xxxxxxxxxx> 
> >>> wrote:
> >>>>
> >>>> On Fri, Apr 17, 2020 at 10:06:31AM -0700, Tamas K Lengyel wrote:
> >>>>> When a forked VM is being reset while having vm_events active, 
> >>>>> re-copying the
> >>>>> vCPU info page can lead to events being lost. This seems to only affect 
> >>>>> a
> >>>>> subset of events (interrupts), while not others (cpuid, MTF, EPT) thus 
> >>>>> it was
> >>>>
> >>>> I'm slightly lost by the sentence, is the guest or the hypervisor
> >>>> the one losing events?
> >>>>
> >>>> Ie: interrupts are events from a guest PoV, but cpuid or EPT is not
> >>>> something that triggers events that are injected to the guest. I think
> >>>> the commit message needs clarification.
> >>>
> >>> Sorry, what I meant was software interrupts are not triggered anymore,
> >>> ie. int3 and it's associated event is not sent to the monitor
> >>> application (VM_EVENT_REASON_SOFTWARE_BREAKPOINT).
> >>>
> >>>>
> >>>>> not discovered beforehand. Only copying vCPU info page contents during 
> >>>>> initial
> >>>>> fork fixes the problem.
> >>>>
> >>>> Hm, I'm not sure I understand why this is causing issues. When you
> >>>> reset a fork you should reset the vcpu info page, or else event masks 
> >>>> would
> >>>> be in a wrong state?
> >>>
> >>> When we reset a fork we only want to 1) discard any memory allocated
> >>> for it 2) reset the vCPU registers. We don't want to reset event
> >>> channels or anything else. We have active vm_events on the domain and
> >>> the whole point of doing a fork reset is to avoid having to
> >>> reinitialize all that as it's quite slow.
> >>
> >> So for an arbitrary piece of state, what are the criteria to establish
> >> whether to copy or re-init them during a fork? Is it really now and
> >> forever only memory that wants resetting? I have to admit I'm confused
> >> by you also mentioning CPU registers - aren't they to be copied rather
> >> than reset?
> >
> > Registers are being reset by copying them from the parent. Allocated
> > memory is discarded as the memory that's needed for the new execution
> > will get copied when EPT faults happen as it's executing. The goal is
> > to put the domain back to its initial execution state without having
> > to reinitialize vm_events. In our experiments when the forks are
> > executed only for a very short period (fuzzing), having to
> > reinitialize the vm_event interfaces mean going from ~100 execution/s
> > to ~2 executions/s. Unfortunately in the current state the fork
> > doesn't generate the required vm_events after the first execution and
> > for some reason it only happens for int3 generated events.
>
> Thanks, but I'm afraid this doesn't answer my question regarding the
> criteria for what should be put back to the fork's initial state vs
> what should be left as is. In fact _anything_ not getting reset to
> initial state would seem to need special justification (beyond
> performance considerations).

>From my PoV everything should be reset as long as it doesn't interfere
with already registered vm_events. The only part that seems to
interfere with the regular flow of events right now is the
vcpu_info_mfn.

I've ran some further tests and seems that when the code that is being
fuzzed is in ring3, int3 events are delivered as expected after a fork
reset. But if the code in question is ring0, then the expected int3 is
not delivered. It could entirely be that in the kernel-mode case the
code takes a detour and the reason we don't see the expected int3 is
not an interference with vm_events directly, rather a crash in the
guest due to the vcpu_info_mfn being reset. In either case, it needs
to be avoided.

Tamas

Tamas