Re: [Xen-devel] Xen Introspection, KPTI, and CR3 bit 63 leads to guest VMENTRY failures during introspection

["Bitweasil ." <bitweasil@xxxxxxxxxxxxxx>]

Razvan -

The proposed changes would only have an impact if CR3 exiting is enabled, which implies a pair of world switches and other code execution in a different region of memory and with different page tables anyway.

Under normal operation, CR3 exiting is not turned on, so this will have no impact on operation.

Are there any non-introspection cases in which CR3 exiting will be enabled for hardware virtualized guests?  Given the time cost of a pair of world switches and handling the associated code, I question if one could even measure the difference of the TLB flush or not.  The CR3 reporting performance hit under KPTI is quite catastrophic anyway, though I expect one could mitigate that somewhat with CR3-targets, if one wanted to.

On Fri, Jan 26, 2018 at 12:39 AM Razvan Cojocaru <rcojocaru@xxxxxxxxxxxxxxx> wrote:

On 01/26/2018 02:02 AM, Bitweasil . wrote:
> This also impacts the "on change only" control setting code.
>
> From some debugging code: hvm_event_cr, value: 800000001aad0000, old:
> 1aad0000
>
> This should not count as a changed CR3 value for reporting purposes.
>
> The following patch resolves this reasonably, for a sane value of
> "change only" reporting.  Apologies for the old Xen version, but I
> assume this is still an issue if the other issue was not resolved in
> staging.
>
>
>
> --- xenserver-4.7.1-clean/xen/arch/x86/hvm/event.c    2017-05-10
> 11:29:14.135332964 -0600
> +++ xenserver-4.7.1/xen/arch/x86/hvm/event.c    2018-01-25
> 16:52:05.938251000 -0700
> @@ -33,6 +33,11 @@
>      struct arch_domain *ad = &curr->domain->arch;
>      unsigned int ctrlreg_bitmask = monitor_ctrlreg_bitmask(index);
>  
> +
> +    // Patch in work from Razvan
> +    if ( hvm_pcid_enabled(curr) )
> +        value &= ((1ull << 63) - 1);
> +
>      if ( (ad->monitor.write_ctrlreg_enabled & ctrlreg_bitmask) &&
>           (!(ad->monitor.write_ctrlreg_onchangeonly & ctrlreg_bitmask) ||
>            value != old) )

Thanks, that's right. But I keep thinking that the proposed changes
would now incur the flush even on non-introspection paths (which for
some reason seem to work fine with the noflush bit set).

So I think this patch will affect all CR3 writes, since the only
difference between introspection and no introspection is that
hvm_set_cr3() is called with may_defer == true in the first case, and
false in the second, and both cases eventually end up clearing noflush.

I this acceptable to everyone, or do we make this smarter?


Thanks,
Razvan

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