Re: [PATCH v1] x86/hvm: Generic instruction re-execution mechanism for execute faults

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

>>> On 27.11.18 at 11:49, <rcojocaru@bitdefender.com> wrote:
> On 11/23/18 11:07 AM, Jan Beulich wrote:
>>>>> On 22.11.18 at 19:24, <rcojocaru@bitdefender.com> wrote:
>>> _However_, please picture an instruction that both writes into a page P1
>>> we're interested in, _and_ causes a write into a read-only page-walk
>>> related page P2. Emulating the current instruction, as the upstream
>>> patch does, does eliminate the vm_event caused by writing into P2, but
>>> with the unfortunate side-effect of losing a potentially critical event
>>> for the write into P1.
>>>
>>> What this patch attempts to do is to mark P1 rwx (so allow the write),
>>> then put the faulting VCPU into singlestep mode, then restore the
>>> restrictions after it has finished single stepping. By now it's obvious
>>> why all the other VCPUs need to be paused: one of them might do a
>>> malicious write into P1 that silently succeeds (since the EPT is shared
>>> among all VCPUs - putting altp2m aside for a moment). We don't want that.
>> 
>> I think this all goes into the fundamentally wrong direction. If lost
>> events during emulation are your issue, then let's make sure
>> emulation paths trigger the same events hardware would.
> 
> It's complicated: we very much like that the emulator is ignoring page
> restrictions - this allows us to proceed with instructions writing into
> protected pages without lifting said restrictions (when those
> instructions are deemed to be safe by the introspection engine). That is
> the most efficient mechanism we have, since we can just reply "emulate"
> to a vm_event and that's it.
> 
> The alternative is to use altp2m, have an unrestricted view (view 0 fits
> the bill nicely since restrictions on in propagate to all other active
> altp2ms), get an EPT fault vm_event, reply with "switch to view 0 and
> put VCPU in single-step mode", then wait for the single step event, and
> then reply "switch back to restricted altp2m view and get the VCPU out
> of single-step mode". Clearly the altp2m option is at least twice as
> slow, so we prefer to emulate the instruction with a single vm_event
> reply and move on.
> 
> A special case is when the emulator doesn't support an instruction, in
> which case we can get an UNIMPLEMENTED vm_event, and switch to altp2m
> just for that. Between these, everything should be covered and the
> guests should run without problems.
> 
> However, we need to get altp2m up to speed, fully working and reliable
> to be able to do all that.
> 
> About the emulator and events: if we could have a toggle for the
> emulator to tell it "emulate the current instruction and send out a
> vm_event only if it touches a protected page that's NOT part of the page
> walk", that would also work - though I can't at this point tell how
> feasible those modifications are.

For the emulation paths it is certainly possible to have controls for
(almost) everything, if needed. So going that route continues to
look more desirable to me than going the route you've chosen.

>> With a sufficiently complete insn emulator, single-stepping should
>> not be needed at all imo. Granted we're not quite there yet with
>> the emulator, but we've made quite a bit of progress. As before,
>> if there are particular instructions you know of that the emulator
>> doesn't handle yet, please keep pointing these out. Last I know
>> were some AVX move instructions, which have long been
>> implemented.
> 
> True, I haven't seen emulator issues in that respect with staging - the
> emulator appears lately to be sufficiently complete. Thank you very much
> for your help and support - we'll definitely point out unsupported
> instructions if we spot some again.
> 
> The bigger practical problem is having something that works with older
> Xen versions - for example current XenServer releases are still on Xen
> 4.7, and backporting the whole emulator machinery there is not
> reasonable, with the large number of changes that have occured in the
> meantime.

Well - wouldn't this apply to extensive altp2m changes as well?

Jan


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