Re: [Xen-devel] [PATCH] x86/vm_event: correctly gather gs_shadow value

[Tamas K Lengyel <tamas@xxxxxxxxxxxxx>]

On Wed, May 1, 2019 at 9:44 AM Razvan Cojocaru
<rcojocaru@xxxxxxxxxxxxxxx> wrote:
>
> On 5/1/19 6:01 PM, Tamas K Lengyel wrote:
> > On Wed, May 1, 2019 at 8:53 AM Tamas K Lengyel <tamas@xxxxxxxxxxxxx> wrote:
> >>
> >> On Wed, May 1, 2019 at 8:20 AM Razvan Cojocaru
> >> <rcojocaru@xxxxxxxxxxxxxxx> wrote:
> >>>
> >>> On 5/1/19 4:58 PM, Tamas K Lengyel wrote:
> >>>>>> It might be worth introducing a "sync state from hw" hook which 
> >>>>>> collects
> >>>>>> all the data we intend to pass to the introspection agent.
> >>>>>
> >>>>> You mean adding another hvm hook?
> >>>>
> >>>> Actually, instead of another hook I think what would make sense it to
> >>>> just update vmx_save_vmcs_ctxt to automatically refresh the cached
> >>>> register values when it's called with "v == current". Thoughts?
> >>>
> >>> That's probably the better way to go about it, since otherwise the
> >>> xc_hvm_getcontext_partial() hypercall will suffer from the same problem.
> >>> (there are two ways of getting guest state: one is via the vm_event
> >>> cached values, the other is via the aforementioned hypercall).
> >>
> >> True, although issuing the hypercall in the vm_event callback is
> >> actually fine - that's how I found the issue to begin with, since the
> >> vCPU will be scheduled out with the cached registers refreshed and
> >> thus be different then what the vm_event itself had. But other callers
> >> of the hypercall can run into the problem if the guest/vcpu is not
> >> paused.
> >
> > Actually, doing the "v == current" check wouldn't really do anything
> > for the hypercall since it's not the domain issuing the hypercall on
> > itself. The only way to ensure that hypercall is returning correct
> > values would be to pause the vCPU.
>
> I've discussed this with Andrew in the meantime and he's kindly pointed
> out that for the hypercall we pause from remote context, which forces a
> de-schedule. So the hypercall _should_ be fine. But we write data to the
> vm_event ring from the current context, where state might actually be in
> hardware.

Correct, I've went through the hypercall code and it does pause the
vCPU. So right now I don't think we need anything else then what's
already in this patch.

Tamas

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