>>> On 18.04.18 at 11:37, <jgr...@suse.com> wrote: > On 18/04/18 11:13, Jan Beulich wrote: >>>>> On 18.04.18 at 10:30, <jgr...@suse.com> wrote: >>> Avoid flushing the complete TLB when switching %cr3 for mitigation of >>> Meltdown by using the PCID feature if available. >>> >>> We are using 4 PCID values for a 64 bit pv domain subject to XPTI and >>> 2 values for the non-XPTI case: >>> >>> - guest active and in kernel mode >>> - guest active and in user mode >>> - hypervisor active and guest in user mode (XPTI only) >>> - hypervisor active and guest in kernel mode (XPTI only) >>> >>> We use PCID only if PCID _and_ INVPCID are supported. With PCID in use >>> we disable global pages in cr4. A command line parameter controls in >>> which cases PCID is being used. >>> >>> As the non-XPTI case has shown not to perform better with PCID at least >>> on some machines the default is to use PCID only for domains subject to >>> XPTI. >>> >>> With PCID enabled we always disable global pages. This avoids having to >>> either flush the complete TLB or do a cycle through all PCID values >>> when invalidating a single global page. >>> >>> Signed-off-by: Juergen Gross <jgr...@suse.com> >>> Reviewed-by: Jan Beulich <jbeul...@suse.com> >>> --- >>> V6.1: >>> - address some minor comments (Jan Beulich) >> >> No v7 changes? Afaict ... >> >>> @@ -717,7 +718,7 @@ int __init dom0_construct_pv(struct domain *d, >>> update_cr3(v); >>> >>> /* We run on dom0's page tables for the final part of the build > process. */ >>> - switch_cr3_cr4(v->arch.cr3, read_cr4()); >>> + switch_cr3_cr4(cr3_pa(v->arch.cr3), read_cr4()); >> >> ... at least this was added. And to be honest I'm not convinced cr3_pa() is >> the right construct to use here: It's neither clear why the other bits don't >> matter at this point, nor why any of the bits that you mask out this way >> need masking out in the first place (e.g. why, in the crash case that Andrew >> had observed, the noflush bit was wrongly set). > > At this point in time we only want to use another cr3 value. So PCIDs > should _not_ be used as this would require adapting cr4, resulting in > writing the cr3_pa() bits only. It would have been possible to use > write_cr3() here, but only together with open coding a TLB flush in > order to avoid any global pages remaining in the TLB. So I decided to > use switch_cr3_cr4(). > > The noflush bit was set as dom0 is subject to XPTI and PCID usage and > update_cr3() is updating v->arch.cr3 accordingly. > > I know you are not fond of setting noflush in v->arch.cr3. The only > sensible alternative to that would be adding something like > v->arch.cr3_pcid_bits being or-ed to the cr3 value in restore_all_guest. > This would let us get rid of having to use cr3_pa() in some places > while adding a (very little) performance penalty.
Well, I had accepted the current approach as the (performance wise) better alternative, but with Andrew sharing that original concern of mine, and with there having been a first real problem resulting from that approach, more seriously considering the alternative certainly seems necessary. Andrew? >> I hope there aren't any other such hidden changes in this version of the >> series. > > Oh sorry, I missed to update the history. > > I'm not aware of other changes in v8 (apart from patch 1 and the > resulting needed change in patch 3). Ah - I hadn't spotted that follow-on change, and I've just sent a small comment in its regard. Jan _______________________________________________ Xen-devel mailing list Xen-devel@lists.xenproject.org https://lists.xenproject.org/mailman/listinfo/xen-devel
