* [PATCH 0/2] Use invpcid @ 2018-03-05 9:50 Wei Liu 2018-03-05 9:50 ` [PATCH 1/2] x86: report if PCID and INVPCID are supported Wei Liu 2018-03-05 9:50 ` [PATCH 2/2] x86: use invpcid to do global flushing Wei Liu 0 siblings, 2 replies; 37+ messages in thread From: Wei Liu @ 2018-03-05 9:50 UTC (permalink / raw) To: Xen-devel; +Cc: Juergen Gross, Andrew Cooper, Wei Liu, Jan Beulich Wei Liu (2): x86: report if PCID and INVPCID are supported x86: use invpcid to do global flushing xen/arch/x86/flushtlb.c | 22 ++++++++++++++++++---- xen/arch/x86/setup.c | 7 +++++++ 2 files changed, 25 insertions(+), 4 deletions(-) -- 2.11.0 _______________________________________________ Xen-devel mailing list Xen-devel@lists.xenproject.org https://lists.xenproject.org/mailman/listinfo/xen-devel ^ permalink raw reply [flat|nested] 37+ messages in thread
* [PATCH 1/2] x86: report if PCID and INVPCID are supported 2018-03-05 9:50 [PATCH 0/2] Use invpcid Wei Liu @ 2018-03-05 9:50 ` Wei Liu 2018-03-05 9:51 ` Juergen Gross ` (2 more replies) 2018-03-05 9:50 ` [PATCH 2/2] x86: use invpcid to do global flushing Wei Liu 1 sibling, 3 replies; 37+ messages in thread From: Wei Liu @ 2018-03-05 9:50 UTC (permalink / raw) To: Xen-devel; +Cc: Juergen Gross, Andrew Cooper, Wei Liu, Jan Beulich Signed-off-by: Wei Liu <wei.liu2@citrix.com> --- xen/arch/x86/setup.c | 7 +++++++ 1 file changed, 7 insertions(+) diff --git a/xen/arch/x86/setup.c b/xen/arch/x86/setup.c index ac530ece2c..89e42865a4 100644 --- a/xen/arch/x86/setup.c +++ b/xen/arch/x86/setup.c @@ -1701,6 +1701,13 @@ void __init noreturn __start_xen(unsigned long mbi_p) cpu_has_nx ? XENLOG_INFO : XENLOG_WARNING "Warning: ", cpu_has_nx ? "" : "not "); + + printk(XENLOG_INFO + "PCID (Process-Context IDentifier) %ssupported\n", + cpu_has_pcid ? "" : "not "); + + printk(XENLOG_INFO "INVPCID %ssupported\n", cpu_has_invpcid ? "" : "not "); + /* * We're going to setup domain0 using the module(s) that we stashed safely * above our heap. The second module, if present, is an initrd ramdisk. -- 2.11.0 _______________________________________________ Xen-devel mailing list Xen-devel@lists.xenproject.org https://lists.xenproject.org/mailman/listinfo/xen-devel ^ permalink raw reply related [flat|nested] 37+ messages in thread
* Re: [PATCH 1/2] x86: report if PCID and INVPCID are supported 2018-03-05 9:50 ` [PATCH 1/2] x86: report if PCID and INVPCID are supported Wei Liu @ 2018-03-05 9:51 ` Juergen Gross 2018-03-05 11:20 ` Jan Beulich [not found] ` <5A9D362002000078001AE74F@suse.com> 2 siblings, 0 replies; 37+ messages in thread From: Juergen Gross @ 2018-03-05 9:51 UTC (permalink / raw) To: Wei Liu, Xen-devel; +Cc: Andrew Cooper, Jan Beulich On 05/03/18 10:50, Wei Liu wrote: > Signed-off-by: Wei Liu <wei.liu2@citrix.com> Reviewed-by: Juergen Gross <jgross@suse.com> Juergen _______________________________________________ Xen-devel mailing list Xen-devel@lists.xenproject.org https://lists.xenproject.org/mailman/listinfo/xen-devel ^ permalink raw reply [flat|nested] 37+ messages in thread
* Re: [PATCH 1/2] x86: report if PCID and INVPCID are supported 2018-03-05 9:50 ` [PATCH 1/2] x86: report if PCID and INVPCID are supported Wei Liu 2018-03-05 9:51 ` Juergen Gross @ 2018-03-05 11:20 ` Jan Beulich 2018-03-05 11:31 ` Wei Liu [not found] ` <5A9D362002000078001AE74F@suse.com> 2 siblings, 1 reply; 37+ messages in thread From: Jan Beulich @ 2018-03-05 11:20 UTC (permalink / raw) To: Wei Liu; +Cc: Juergen Gross, Andrew Cooper, Xen-devel >>> On 05.03.18 at 10:50, <wei.liu2@citrix.com> wrote: > --- a/xen/arch/x86/setup.c > +++ b/xen/arch/x86/setup.c > @@ -1701,6 +1701,13 @@ void __init noreturn __start_xen(unsigned long mbi_p) > cpu_has_nx ? XENLOG_INFO : XENLOG_WARNING "Warning: ", > cpu_has_nx ? "" : "not "); > > + > + printk(XENLOG_INFO > + "PCID (Process-Context IDentifier) %ssupported\n", > + cpu_has_pcid ? "" : "not "); > + > + printk(XENLOG_INFO "INVPCID %ssupported\n", cpu_has_invpcid ? "" : "not "); Do we really need this? We log a message for NX as an exception, we don't do so for other features (and things would get pretty unwieldy if we did). Jan _______________________________________________ Xen-devel mailing list Xen-devel@lists.xenproject.org https://lists.xenproject.org/mailman/listinfo/xen-devel ^ permalink raw reply [flat|nested] 37+ messages in thread
* Re: [PATCH 1/2] x86: report if PCID and INVPCID are supported 2018-03-05 11:20 ` Jan Beulich @ 2018-03-05 11:31 ` Wei Liu 2018-03-05 11:48 ` Andrew Cooper 0 siblings, 1 reply; 37+ messages in thread From: Wei Liu @ 2018-03-05 11:31 UTC (permalink / raw) To: Jan Beulich; +Cc: Juergen Gross, Andrew Cooper, Wei Liu, Xen-devel On Mon, Mar 05, 2018 at 04:20:48AM -0700, Jan Beulich wrote: > >>> On 05.03.18 at 10:50, <wei.liu2@citrix.com> wrote: > > --- a/xen/arch/x86/setup.c > > +++ b/xen/arch/x86/setup.c > > @@ -1701,6 +1701,13 @@ void __init noreturn __start_xen(unsigned long mbi_p) > > cpu_has_nx ? XENLOG_INFO : XENLOG_WARNING "Warning: ", > > cpu_has_nx ? "" : "not "); > > > > + > > + printk(XENLOG_INFO > > + "PCID (Process-Context IDentifier) %ssupported\n", > > + cpu_has_pcid ? "" : "not "); > > + > > + printk(XENLOG_INFO "INVPCID %ssupported\n", cpu_has_invpcid ? "" : "not "); > > Do we really need this? We log a message for NX as an exception, > we don't do so for other features (and things would get pretty > unwieldy if we did). > I'm not too fuss really. It helped me to pick a machine when I didn't have the exact spec of the pool of machines. The same information can certainly be obtained from hardware cpuid info. On another note, what is the most reliable way to extract hardware cpuid info from Xen? AIUI now cpuid faulting can be enabled for both PV and HVM guests so running cpuid in PV guests might not return the real bits (which is a good thing). Wei. _______________________________________________ Xen-devel mailing list Xen-devel@lists.xenproject.org https://lists.xenproject.org/mailman/listinfo/xen-devel ^ permalink raw reply [flat|nested] 37+ messages in thread
* Re: [PATCH 1/2] x86: report if PCID and INVPCID are supported 2018-03-05 11:31 ` Wei Liu @ 2018-03-05 11:48 ` Andrew Cooper 0 siblings, 0 replies; 37+ messages in thread From: Andrew Cooper @ 2018-03-05 11:48 UTC (permalink / raw) To: Wei Liu, Jan Beulich; +Cc: Juergen Gross, Xen-devel On 05/03/18 11:31, Wei Liu wrote: > On Mon, Mar 05, 2018 at 04:20:48AM -0700, Jan Beulich wrote: >>>>> On 05.03.18 at 10:50, <wei.liu2@citrix.com> wrote: >>> --- a/xen/arch/x86/setup.c >>> +++ b/xen/arch/x86/setup.c >>> @@ -1701,6 +1701,13 @@ void __init noreturn __start_xen(unsigned long mbi_p) >>> cpu_has_nx ? XENLOG_INFO : XENLOG_WARNING "Warning: ", >>> cpu_has_nx ? "" : "not "); >>> >>> + >>> + printk(XENLOG_INFO >>> + "PCID (Process-Context IDentifier) %ssupported\n", >>> + cpu_has_pcid ? "" : "not "); >>> + >>> + printk(XENLOG_INFO "INVPCID %ssupported\n", cpu_has_invpcid ? "" : "not "); >> Do we really need this? We log a message for NX as an exception, >> we don't do so for other features (and things would get pretty >> unwieldy if we did). >> > I'm not too fuss really. It helped me to pick a machine when I didn't > have the exact spec of the pool of machines. The same information can > certainly be obtained from hardware cpuid info. > > On another note, what is the most reliable way to extract hardware cpuid > info from Xen? AIUI now cpuid faulting can be enabled for both PV and > HVM guests so running cpuid in PV guests might not return the real bits > (which is a good thing). For now, `xen-cpuid`, which uses SYSCTL_get_cpufeatureset internally. This will shortly be changing to get the fully cpuid/msr policies rather than just the feature bitmaps. ~Andrew _______________________________________________ Xen-devel mailing list Xen-devel@lists.xenproject.org https://lists.xenproject.org/mailman/listinfo/xen-devel ^ permalink raw reply [flat|nested] 37+ messages in thread
[parent not found: <5A9D362002000078001AE74F@suse.com>]
* Re: [PATCH 1/2] x86: report if PCID and INVPCID are supported [not found] ` <5A9D362002000078001AE74F@suse.com> @ 2018-03-05 11:43 ` Juergen Gross 2018-03-05 12:37 ` Jan Beulich [not found] ` <5A9D482202000078001AE7ED@suse.com> 0 siblings, 2 replies; 37+ messages in thread From: Juergen Gross @ 2018-03-05 11:43 UTC (permalink / raw) To: Jan Beulich, Wei Liu; +Cc: Andrew Cooper, Xen-devel On 05/03/18 12:20, Jan Beulich wrote: >>>> On 05.03.18 at 10:50, <wei.liu2@citrix.com> wrote: >> --- a/xen/arch/x86/setup.c >> +++ b/xen/arch/x86/setup.c >> @@ -1701,6 +1701,13 @@ void __init noreturn __start_xen(unsigned long mbi_p) >> cpu_has_nx ? XENLOG_INFO : XENLOG_WARNING "Warning: ", >> cpu_has_nx ? "" : "not "); >> >> + >> + printk(XENLOG_INFO >> + "PCID (Process-Context IDentifier) %ssupported\n", >> + cpu_has_pcid ? "" : "not "); >> + >> + printk(XENLOG_INFO "INVPCID %ssupported\n", cpu_has_invpcid ? "" : "not "); > > Do we really need this? We log a message for NX as an exception, > we don't do so for other features (and things would get pretty > unwieldy if we did). I'd rather keep this message. As we are hiding PCID and INPCID from dom0 this is the only indicator of those features being supported. In case of an error related to TLB consistency this information is important IMO. Juergen _______________________________________________ Xen-devel mailing list Xen-devel@lists.xenproject.org https://lists.xenproject.org/mailman/listinfo/xen-devel ^ permalink raw reply [flat|nested] 37+ messages in thread
* Re: [PATCH 1/2] x86: report if PCID and INVPCID are supported 2018-03-05 11:43 ` Juergen Gross @ 2018-03-05 12:37 ` Jan Beulich [not found] ` <5A9D482202000078001AE7ED@suse.com> 1 sibling, 0 replies; 37+ messages in thread From: Jan Beulich @ 2018-03-05 12:37 UTC (permalink / raw) To: Wei Liu, Juergen Gross; +Cc: Andrew Cooper, Xen-devel >>> On 05.03.18 at 12:43, <jgross@suse.com> wrote: > On 05/03/18 12:20, Jan Beulich wrote: >>>>> On 05.03.18 at 10:50, <wei.liu2@citrix.com> wrote: >>> --- a/xen/arch/x86/setup.c >>> +++ b/xen/arch/x86/setup.c >>> @@ -1701,6 +1701,13 @@ void __init noreturn __start_xen(unsigned long mbi_p) >>> cpu_has_nx ? XENLOG_INFO : XENLOG_WARNING "Warning: ", >>> cpu_has_nx ? "" : "not "); >>> >>> + >>> + printk(XENLOG_INFO >>> + "PCID (Process-Context IDentifier) %ssupported\n", >>> + cpu_has_pcid ? "" : "not "); >>> + >>> + printk(XENLOG_INFO "INVPCID %ssupported\n", cpu_has_invpcid ? "" : "not "); >> >> Do we really need this? We log a message for NX as an exception, >> we don't do so for other features (and things would get pretty >> unwieldy if we did). > > I'd rather keep this message. As we are hiding PCID and INPCID from dom0 > this is the only indicator of those features being supported. In case of > an error related to TLB consistency this information is important IMO. Well, this argument would hold for almost every other advanced feature we use. Jan _______________________________________________ Xen-devel mailing list Xen-devel@lists.xenproject.org https://lists.xenproject.org/mailman/listinfo/xen-devel ^ permalink raw reply [flat|nested] 37+ messages in thread
[parent not found: <5A9D482202000078001AE7ED@suse.com>]
* Re: [PATCH 1/2] x86: report if PCID and INVPCID are supported [not found] ` <5A9D482202000078001AE7ED@suse.com> @ 2018-03-05 12:49 ` Juergen Gross 2018-03-05 12:57 ` Jan Beulich 0 siblings, 1 reply; 37+ messages in thread From: Juergen Gross @ 2018-03-05 12:49 UTC (permalink / raw) To: Jan Beulich, Wei Liu; +Cc: Andrew Cooper, Xen-devel On 05/03/18 13:37, Jan Beulich wrote: >>>> On 05.03.18 at 12:43, <jgross@suse.com> wrote: >> On 05/03/18 12:20, Jan Beulich wrote: >>>>>> On 05.03.18 at 10:50, <wei.liu2@citrix.com> wrote: >>>> --- a/xen/arch/x86/setup.c >>>> +++ b/xen/arch/x86/setup.c >>>> @@ -1701,6 +1701,13 @@ void __init noreturn __start_xen(unsigned long mbi_p) >>>> cpu_has_nx ? XENLOG_INFO : XENLOG_WARNING "Warning: ", >>>> cpu_has_nx ? "" : "not "); >>>> >>>> + >>>> + printk(XENLOG_INFO >>>> + "PCID (Process-Context IDentifier) %ssupported\n", >>>> + cpu_has_pcid ? "" : "not "); >>>> + >>>> + printk(XENLOG_INFO "INVPCID %ssupported\n", cpu_has_invpcid ? "" : "not "); >>> >>> Do we really need this? We log a message for NX as an exception, >>> we don't do so for other features (and things would get pretty >>> unwieldy if we did). >> >> I'd rather keep this message. As we are hiding PCID and INPCID from dom0 >> this is the only indicator of those features being supported. In case of >> an error related to TLB consistency this information is important IMO. > > Well, this argument would hold for almost every other advanced > feature we use. Then I'd suggest to either add individual messages for those features or to have a single message (or a few) to list all features like the flags in /proc/cpuinfo. This list could even be part of the "xl info" output. Juergen _______________________________________________ Xen-devel mailing list Xen-devel@lists.xenproject.org https://lists.xenproject.org/mailman/listinfo/xen-devel ^ permalink raw reply [flat|nested] 37+ messages in thread
* Re: [PATCH 1/2] x86: report if PCID and INVPCID are supported 2018-03-05 12:49 ` Juergen Gross @ 2018-03-05 12:57 ` Jan Beulich 2018-03-05 13:26 ` Wei Liu 0 siblings, 1 reply; 37+ messages in thread From: Jan Beulich @ 2018-03-05 12:57 UTC (permalink / raw) To: Wei Liu, Juergen Gross; +Cc: Andrew Cooper, Xen-devel >>> On 05.03.18 at 13:49, <jgross@suse.com> wrote: > On 05/03/18 13:37, Jan Beulich wrote: >>>>> On 05.03.18 at 12:43, <jgross@suse.com> wrote: >>> On 05/03/18 12:20, Jan Beulich wrote: >>>>>>> On 05.03.18 at 10:50, <wei.liu2@citrix.com> wrote: >>>>> --- a/xen/arch/x86/setup.c >>>>> +++ b/xen/arch/x86/setup.c >>>>> @@ -1701,6 +1701,13 @@ void __init noreturn __start_xen(unsigned long mbi_p) >>>>> cpu_has_nx ? XENLOG_INFO : XENLOG_WARNING "Warning: ", >>>>> cpu_has_nx ? "" : "not "); >>>>> >>>>> + >>>>> + printk(XENLOG_INFO >>>>> + "PCID (Process-Context IDentifier) %ssupported\n", >>>>> + cpu_has_pcid ? "" : "not "); >>>>> + >>>>> + printk(XENLOG_INFO "INVPCID %ssupported\n", cpu_has_invpcid ? "" : "not "); >>>> >>>> Do we really need this? We log a message for NX as an exception, >>>> we don't do so for other features (and things would get pretty >>>> unwieldy if we did). >>> >>> I'd rather keep this message. As we are hiding PCID and INPCID from dom0 >>> this is the only indicator of those features being supported. In case of >>> an error related to TLB consistency this information is important IMO. >> >> Well, this argument would hold for almost every other advanced >> feature we use. > > Then I'd suggest to either add individual messages for those features or > to have a single message (or a few) to list all features like the flags > in /proc/cpuinfo. This list could even be part of the "xl info" output. I have no idea whether there are plans to make this part of "xl info" output; xen-cpuid certainly can tell you. Wei, btw - for the case of there being problems with this new use of INVPCID, perhaps you will also want to extend parse_xen_cpuid() so that the use can be disabled from the command line. Jan _______________________________________________ Xen-devel mailing list Xen-devel@lists.xenproject.org https://lists.xenproject.org/mailman/listinfo/xen-devel ^ permalink raw reply [flat|nested] 37+ messages in thread
* Re: [PATCH 1/2] x86: report if PCID and INVPCID are supported 2018-03-05 12:57 ` Jan Beulich @ 2018-03-05 13:26 ` Wei Liu 0 siblings, 0 replies; 37+ messages in thread From: Wei Liu @ 2018-03-05 13:26 UTC (permalink / raw) To: Jan Beulich; +Cc: Juergen Gross, Andrew Cooper, Wei Liu, Xen-devel On Mon, Mar 05, 2018 at 05:57:48AM -0700, Jan Beulich wrote: > >>> On 05.03.18 at 13:49, <jgross@suse.com> wrote: > > On 05/03/18 13:37, Jan Beulich wrote: > >>>>> On 05.03.18 at 12:43, <jgross@suse.com> wrote: > >>> On 05/03/18 12:20, Jan Beulich wrote: > >>>>>>> On 05.03.18 at 10:50, <wei.liu2@citrix.com> wrote: > >>>>> --- a/xen/arch/x86/setup.c > >>>>> +++ b/xen/arch/x86/setup.c > >>>>> @@ -1701,6 +1701,13 @@ void __init noreturn __start_xen(unsigned long mbi_p) > >>>>> cpu_has_nx ? XENLOG_INFO : XENLOG_WARNING "Warning: ", > >>>>> cpu_has_nx ? "" : "not "); > >>>>> > >>>>> + > >>>>> + printk(XENLOG_INFO > >>>>> + "PCID (Process-Context IDentifier) %ssupported\n", > >>>>> + cpu_has_pcid ? "" : "not "); > >>>>> + > >>>>> + printk(XENLOG_INFO "INVPCID %ssupported\n", cpu_has_invpcid ? "" : "not "); > >>>> > >>>> Do we really need this? We log a message for NX as an exception, > >>>> we don't do so for other features (and things would get pretty > >>>> unwieldy if we did). > >>> > >>> I'd rather keep this message. As we are hiding PCID and INPCID from dom0 > >>> this is the only indicator of those features being supported. In case of > >>> an error related to TLB consistency this information is important IMO. > >> > >> Well, this argument would hold for almost every other advanced > >> feature we use. > > > > Then I'd suggest to either add individual messages for those features or > > to have a single message (or a few) to list all features like the flags > > in /proc/cpuinfo. This list could even be part of the "xl info" output. > > I have no idea whether there are plans to make this part of > "xl info" output; xen-cpuid certainly can tell you. > > Wei, btw - for the case of there being problems with this new use > of INVPCID, perhaps you will also want to extend parse_xen_cpuid() > so that the use can be disabled from the command line. > Noted. _______________________________________________ Xen-devel mailing list Xen-devel@lists.xenproject.org https://lists.xenproject.org/mailman/listinfo/xen-devel ^ permalink raw reply [flat|nested] 37+ messages in thread
* [PATCH 2/2] x86: use invpcid to do global flushing 2018-03-05 9:50 [PATCH 0/2] Use invpcid Wei Liu 2018-03-05 9:50 ` [PATCH 1/2] x86: report if PCID and INVPCID are supported Wei Liu @ 2018-03-05 9:50 ` Wei Liu 2018-03-05 9:52 ` Juergen Gross ` (3 more replies) 1 sibling, 4 replies; 37+ messages in thread From: Wei Liu @ 2018-03-05 9:50 UTC (permalink / raw) To: Xen-devel; +Cc: Juergen Gross, Andrew Cooper, Wei Liu, Jan Beulich Signed-off-by: Wei Liu <wei.liu2@citrix.com> --- xen/arch/x86/flushtlb.c | 22 ++++++++++++++++++---- 1 file changed, 18 insertions(+), 4 deletions(-) diff --git a/xen/arch/x86/flushtlb.c b/xen/arch/x86/flushtlb.c index 8a7a76b8ff..e4ea4f3297 100644 --- a/xen/arch/x86/flushtlb.c +++ b/xen/arch/x86/flushtlb.c @@ -9,6 +9,7 @@ #include <xen/sched.h> #include <xen/softirq.h> +#include <asm/invpcid.h> #include <asm/flushtlb.h> #include <asm/page.h> @@ -120,11 +121,24 @@ unsigned int flush_area_local(const void *va, unsigned int flags) else { u32 t = pre_flush(); - unsigned long cr4 = read_cr4(); - write_cr4(cr4 & ~X86_CR4_PGE); - barrier(); - write_cr4(cr4); + if ( !cpu_has_invpcid ) + { + unsigned long cr4 = read_cr4(); + + write_cr4(cr4 & ~X86_CR4_PGE); + barrier(); + write_cr4(cr4); + } + else + { + /* + * Using invpcid to flush all mappings works + * regardless of whether PCID is enabled or not. + * It is faster than read-modify-write CR4. + */ + invpcid_flush_all(); + } post_flush(t); } -- 2.11.0 _______________________________________________ Xen-devel mailing list Xen-devel@lists.xenproject.org https://lists.xenproject.org/mailman/listinfo/xen-devel ^ permalink raw reply related [flat|nested] 37+ messages in thread
* Re: [PATCH 2/2] x86: use invpcid to do global flushing 2018-03-05 9:50 ` [PATCH 2/2] x86: use invpcid to do global flushing Wei Liu @ 2018-03-05 9:52 ` Juergen Gross 2018-03-05 11:31 ` Jan Beulich ` (2 subsequent siblings) 3 siblings, 0 replies; 37+ messages in thread From: Juergen Gross @ 2018-03-05 9:52 UTC (permalink / raw) To: Wei Liu, Xen-devel; +Cc: Andrew Cooper, Jan Beulich On 05/03/18 10:50, Wei Liu wrote: > Signed-off-by: Wei Liu <wei.liu2@citrix.com> Reviewed-by: Juergen Gross <jgross@suse.com> Juergen _______________________________________________ Xen-devel mailing list Xen-devel@lists.xenproject.org https://lists.xenproject.org/mailman/listinfo/xen-devel ^ permalink raw reply [flat|nested] 37+ messages in thread
* Re: [PATCH 2/2] x86: use invpcid to do global flushing 2018-03-05 9:50 ` [PATCH 2/2] x86: use invpcid to do global flushing Wei Liu 2018-03-05 9:52 ` Juergen Gross @ 2018-03-05 11:31 ` Jan Beulich 2018-03-05 11:50 ` Andrew Cooper 2018-03-09 15:29 ` Jan Beulich [not found] ` <5AA2B67302000078001B0567@suse.com> 3 siblings, 1 reply; 37+ messages in thread From: Jan Beulich @ 2018-03-05 11:31 UTC (permalink / raw) To: Wei Liu; +Cc: Juergen Gross, Andrew Cooper, Xen-devel >>> On 05.03.18 at 10:50, <wei.liu2@citrix.com> wrote: > Signed-off-by: Wei Liu <wei.liu2@citrix.com> No description at all? I'd at least expect mention of how much of a performance win this is (for whichever hardware you happen to know that). > @@ -120,11 +121,24 @@ unsigned int flush_area_local(const void *va, unsigned int flags) > else > { > u32 t = pre_flush(); > - unsigned long cr4 = read_cr4(); > > - write_cr4(cr4 & ~X86_CR4_PGE); > - barrier(); > - write_cr4(cr4); > + if ( !cpu_has_invpcid ) > + { > + unsigned long cr4 = read_cr4(); > + > + write_cr4(cr4 & ~X86_CR4_PGE); > + barrier(); > + write_cr4(cr4); > + } > + else > + { > + /* > + * Using invpcid to flush all mappings works > + * regardless of whether PCID is enabled or not. > + * It is faster than read-modify-write CR4. > + */ > + invpcid_flush_all(); > + } The reference to PCID in the comment isn't really meaningful imo. PCID and INVPCID are independent features anyway. Also please don't create artificially short comment lines. Generally I also think such if() conditions would better be inverted: There's no reason to make the legacy form look as if it was preferred. And then - what about the use in write_cr3() and the two uses that remain after my XPTI follow-up series (which sadly looks to be stuck for whatever reason), or (without that series) the write_cr3 assembler macro? Jan _______________________________________________ Xen-devel mailing list Xen-devel@lists.xenproject.org https://lists.xenproject.org/mailman/listinfo/xen-devel ^ permalink raw reply [flat|nested] 37+ messages in thread
* Re: [PATCH 2/2] x86: use invpcid to do global flushing 2018-03-05 11:31 ` Jan Beulich @ 2018-03-05 11:50 ` Andrew Cooper 2018-03-05 12:06 ` Juergen Gross 0 siblings, 1 reply; 37+ messages in thread From: Andrew Cooper @ 2018-03-05 11:50 UTC (permalink / raw) To: Jan Beulich, Wei Liu; +Cc: Juergen Gross, Xen-devel On 05/03/18 11:31, Jan Beulich wrote: >>>> On 05.03.18 at 10:50, <wei.liu2@citrix.com> wrote: >> Signed-off-by: Wei Liu <wei.liu2@citrix.com> > No description at all? I'd at least expect mention of how much of a > performance win this is (for whichever hardware you happen to > know that). > >> @@ -120,11 +121,24 @@ unsigned int flush_area_local(const void *va, unsigned int flags) >> else >> { >> u32 t = pre_flush(); >> - unsigned long cr4 = read_cr4(); >> >> - write_cr4(cr4 & ~X86_CR4_PGE); >> - barrier(); >> - write_cr4(cr4); >> + if ( !cpu_has_invpcid ) >> + { >> + unsigned long cr4 = read_cr4(); >> + >> + write_cr4(cr4 & ~X86_CR4_PGE); >> + barrier(); >> + write_cr4(cr4); >> + } >> + else >> + { >> + /* >> + * Using invpcid to flush all mappings works >> + * regardless of whether PCID is enabled or not. >> + * It is faster than read-modify-write CR4. >> + */ Its a cr4 double write, rather than RMW. We read from a cached value anyway, not from hardware. >> + invpcid_flush_all(); >> + } > The reference to PCID in the comment isn't really meaningful imo. > PCID and INVPCID are independent features anyway. Also please > don't create artificially short comment lines. > > Generally I also think such if() conditions would better be inverted: > There's no reason to make the legacy form look as if it was > preferred. > > And then - what about the use in write_cr3() and the two uses that > remain after my XPTI follow-up series (which sadly looks to be stuck > for whatever reason), or (without that series) the write_cr3 > assembler macro? I don't think it is safe to use invpcid when we're also switching cr3. The new cr3 may have global pages with different translations, as they are guest controlled. (In fact - I'm considering using this property to try and allow us to deliberately trigger MCEs on demand, for testing purposes). ~Andrew _______________________________________________ Xen-devel mailing list Xen-devel@lists.xenproject.org https://lists.xenproject.org/mailman/listinfo/xen-devel ^ permalink raw reply [flat|nested] 37+ messages in thread
* Re: [PATCH 2/2] x86: use invpcid to do global flushing 2018-03-05 11:50 ` Andrew Cooper @ 2018-03-05 12:06 ` Juergen Gross 2018-03-05 12:35 ` Andrew Cooper 0 siblings, 1 reply; 37+ messages in thread From: Juergen Gross @ 2018-03-05 12:06 UTC (permalink / raw) To: Andrew Cooper, Jan Beulich, Wei Liu; +Cc: Xen-devel On 05/03/18 12:50, Andrew Cooper wrote: > On 05/03/18 11:31, Jan Beulich wrote: >>>>> On 05.03.18 at 10:50, <wei.liu2@citrix.com> wrote: >>> Signed-off-by: Wei Liu <wei.liu2@citrix.com> >> No description at all? I'd at least expect mention of how much of a >> performance win this is (for whichever hardware you happen to >> know that). >> >>> @@ -120,11 +121,24 @@ unsigned int flush_area_local(const void *va, unsigned int flags) >>> else >>> { >>> u32 t = pre_flush(); >>> - unsigned long cr4 = read_cr4(); >>> >>> - write_cr4(cr4 & ~X86_CR4_PGE); >>> - barrier(); >>> - write_cr4(cr4); >>> + if ( !cpu_has_invpcid ) >>> + { >>> + unsigned long cr4 = read_cr4(); >>> + >>> + write_cr4(cr4 & ~X86_CR4_PGE); >>> + barrier(); >>> + write_cr4(cr4); >>> + } >>> + else >>> + { >>> + /* >>> + * Using invpcid to flush all mappings works >>> + * regardless of whether PCID is enabled or not. >>> + * It is faster than read-modify-write CR4. >>> + */ > > Its a cr4 double write, rather than RMW. We read from a cached value > anyway, not from hardware. > >>> + invpcid_flush_all(); >>> + } >> The reference to PCID in the comment isn't really meaningful imo. >> PCID and INVPCID are independent features anyway. Also please >> don't create artificially short comment lines. >> >> Generally I also think such if() conditions would better be inverted: >> There's no reason to make the legacy form look as if it was >> preferred. >> >> And then - what about the use in write_cr3() and the two uses that >> remain after my XPTI follow-up series (which sadly looks to be stuck >> for whatever reason), or (without that series) the write_cr3 >> assembler macro? > > I don't think it is safe to use invpcid when we're also switching cr3. > The new cr3 may have global pages with different translations, as they > are guest controlled. Can you elaborate a little bit more? How can a guest control any hypervisor mappings? As long as the new cr3 is being loaded before the TLB is flushed via INVPCID I can't see how a problem should occur. In fact my series does exactly what Jan is asking above: it is replacing the remaining cr4 based TLB flushing by INVPCID if possible. So in case there is a flaw in my design please tell me. Juergen _______________________________________________ Xen-devel mailing list Xen-devel@lists.xenproject.org https://lists.xenproject.org/mailman/listinfo/xen-devel ^ permalink raw reply [flat|nested] 37+ messages in thread
* Re: [PATCH 2/2] x86: use invpcid to do global flushing 2018-03-05 12:06 ` Juergen Gross @ 2018-03-05 12:35 ` Andrew Cooper 2018-03-05 12:54 ` Jan Beulich 2018-03-06 7:10 ` Juergen Gross 0 siblings, 2 replies; 37+ messages in thread From: Andrew Cooper @ 2018-03-05 12:35 UTC (permalink / raw) To: Juergen Gross, Jan Beulich, Wei Liu; +Cc: Xen-devel On 05/03/18 12:06, Juergen Gross wrote: > On 05/03/18 12:50, Andrew Cooper wrote: >> On 05/03/18 11:31, Jan Beulich wrote: >>>>>> On 05.03.18 at 10:50, <wei.liu2@citrix.com> wrote: >>>> Signed-off-by: Wei Liu <wei.liu2@citrix.com> >>> No description at all? I'd at least expect mention of how much of a >>> performance win this is (for whichever hardware you happen to >>> know that). >>> >>>> @@ -120,11 +121,24 @@ unsigned int flush_area_local(const void *va, unsigned int flags) >>>> else >>>> { >>>> u32 t = pre_flush(); >>>> - unsigned long cr4 = read_cr4(); >>>> >>>> - write_cr4(cr4 & ~X86_CR4_PGE); >>>> - barrier(); >>>> - write_cr4(cr4); >>>> + if ( !cpu_has_invpcid ) >>>> + { >>>> + unsigned long cr4 = read_cr4(); >>>> + >>>> + write_cr4(cr4 & ~X86_CR4_PGE); >>>> + barrier(); >>>> + write_cr4(cr4); >>>> + } >>>> + else >>>> + { >>>> + /* >>>> + * Using invpcid to flush all mappings works >>>> + * regardless of whether PCID is enabled or not. >>>> + * It is faster than read-modify-write CR4. >>>> + */ >> Its a cr4 double write, rather than RMW. We read from a cached value >> anyway, not from hardware. >> >>>> + invpcid_flush_all(); >>>> + } >>> The reference to PCID in the comment isn't really meaningful imo. >>> PCID and INVPCID are independent features anyway. Also please >>> don't create artificially short comment lines. >>> >>> Generally I also think such if() conditions would better be inverted: >>> There's no reason to make the legacy form look as if it was >>> preferred. >>> >>> And then - what about the use in write_cr3() and the two uses that >>> remain after my XPTI follow-up series (which sadly looks to be stuck >>> for whatever reason), or (without that series) the write_cr3 >>> assembler macro? >> I don't think it is safe to use invpcid when we're also switching cr3. >> The new cr3 may have global pages with different translations, as they >> are guest controlled. > Can you elaborate a little bit more? > > How can a guest control any hypervisor mappings? As long as the new cr3 > is being loaded before the TLB is flushed via INVPCID I can't see how > a problem should occur. > > In fact my series does exactly what Jan is asking above: it is replacing > the remaining cr4 based TLB flushing by INVPCID if possible. So in case > there is a flaw in my design please tell me. At the moment, we have guest and hypervisor controlled global mappings. The current switch is: cr4 &= ~PGE; cr3 = new_cr3; cr4 |= PGE; which means that all global mappings are flushed by the first action, and no new global mappings can come into existence. We then switch to the new cr3 (again with global fully disabled), then allow global mappings to come back into existence. With the invpcid route, we switch via: cr3 = new_cr3; invpcid all+global; This has a race window where global mappings are active, and could mismatch what is in cr3. This yields #MC on at least some hardware, and is specified to have undefined behaviour. ~Andrew _______________________________________________ Xen-devel mailing list Xen-devel@lists.xenproject.org https://lists.xenproject.org/mailman/listinfo/xen-devel ^ permalink raw reply [flat|nested] 37+ messages in thread
* Re: [PATCH 2/2] x86: use invpcid to do global flushing 2018-03-05 12:35 ` Andrew Cooper @ 2018-03-05 12:54 ` Jan Beulich 2018-03-05 12:57 ` Andrew Cooper 2018-03-06 7:10 ` Juergen Gross 1 sibling, 1 reply; 37+ messages in thread From: Jan Beulich @ 2018-03-05 12:54 UTC (permalink / raw) To: Andrew Cooper, Wei Liu, Juergen Gross; +Cc: Xen-devel >>> On 05.03.18 at 13:35, <andrew.cooper3@citrix.com> wrote: > On 05/03/18 12:06, Juergen Gross wrote: >> On 05/03/18 12:50, Andrew Cooper wrote: >>> On 05/03/18 11:31, Jan Beulich wrote: >>>>>>> On 05.03.18 at 10:50, <wei.liu2@citrix.com> wrote: >>>>> Signed-off-by: Wei Liu <wei.liu2@citrix.com> >>>> No description at all? I'd at least expect mention of how much of a >>>> performance win this is (for whichever hardware you happen to >>>> know that). >>>> >>>>> @@ -120,11 +121,24 @@ unsigned int flush_area_local(const void *va, unsigned > int flags) >>>>> else >>>>> { >>>>> u32 t = pre_flush(); >>>>> - unsigned long cr4 = read_cr4(); >>>>> >>>>> - write_cr4(cr4 & ~X86_CR4_PGE); >>>>> - barrier(); >>>>> - write_cr4(cr4); >>>>> + if ( !cpu_has_invpcid ) >>>>> + { >>>>> + unsigned long cr4 = read_cr4(); >>>>> + >>>>> + write_cr4(cr4 & ~X86_CR4_PGE); >>>>> + barrier(); >>>>> + write_cr4(cr4); >>>>> + } >>>>> + else >>>>> + { >>>>> + /* >>>>> + * Using invpcid to flush all mappings works >>>>> + * regardless of whether PCID is enabled or not. >>>>> + * It is faster than read-modify-write CR4. >>>>> + */ >>> Its a cr4 double write, rather than RMW. We read from a cached value >>> anyway, not from hardware. >>> >>>>> + invpcid_flush_all(); >>>>> + } >>>> The reference to PCID in the comment isn't really meaningful imo. >>>> PCID and INVPCID are independent features anyway. Also please >>>> don't create artificially short comment lines. >>>> >>>> Generally I also think such if() conditions would better be inverted: >>>> There's no reason to make the legacy form look as if it was >>>> preferred. >>>> >>>> And then - what about the use in write_cr3() and the two uses that >>>> remain after my XPTI follow-up series (which sadly looks to be stuck >>>> for whatever reason), or (without that series) the write_cr3 >>>> assembler macro? >>> I don't think it is safe to use invpcid when we're also switching cr3. >>> The new cr3 may have global pages with different translations, as they >>> are guest controlled. >> Can you elaborate a little bit more? >> >> How can a guest control any hypervisor mappings? As long as the new cr3 >> is being loaded before the TLB is flushed via INVPCID I can't see how >> a problem should occur. >> >> In fact my series does exactly what Jan is asking above: it is replacing >> the remaining cr4 based TLB flushing by INVPCID if possible. So in case >> there is a flaw in my design please tell me. > > At the moment, we have guest and hypervisor controlled global mappings. > > The current switch is: > cr4 &= ~PGE; > cr3 = new_cr3; > cr4 |= PGE; > > which means that all global mappings are flushed by the first action, > and no new global mappings can come into existence. We then switch to > the new cr3 (again with global fully disabled), then allow global > mappings to come back into existence. > > With the invpcid route, we switch via: > > cr3 = new_cr3; > invpcid all+global; > > This has a race window where global mappings are active, and could > mismatch what is in cr3. This yields #MC on at least some hardware, and > is specified to have undefined behaviour. Oh, right, this would be okay only without what used to be named USER_MAPPINGS_ARE_GLOBAL (and what is now implied). Jan _______________________________________________ Xen-devel mailing list Xen-devel@lists.xenproject.org https://lists.xenproject.org/mailman/listinfo/xen-devel ^ permalink raw reply [flat|nested] 37+ messages in thread
* Re: [PATCH 2/2] x86: use invpcid to do global flushing 2018-03-05 12:54 ` Jan Beulich @ 2018-03-05 12:57 ` Andrew Cooper 2018-03-05 13:11 ` Juergen Gross 0 siblings, 1 reply; 37+ messages in thread From: Andrew Cooper @ 2018-03-05 12:57 UTC (permalink / raw) To: Jan Beulich, Wei Liu, Juergen Gross; +Cc: Xen-devel On 05/03/18 12:54, Jan Beulich wrote: >>>> On 05.03.18 at 13:35, <andrew.cooper3@citrix.com> wrote: >> On 05/03/18 12:06, Juergen Gross wrote: >>> On 05/03/18 12:50, Andrew Cooper wrote: >>>> On 05/03/18 11:31, Jan Beulich wrote: >>>>>>>> On 05.03.18 at 10:50, <wei.liu2@citrix.com> wrote: >>>>>> Signed-off-by: Wei Liu <wei.liu2@citrix.com> >>>>> No description at all? I'd at least expect mention of how much of a >>>>> performance win this is (for whichever hardware you happen to >>>>> know that). >>>>> >>>>>> @@ -120,11 +121,24 @@ unsigned int flush_area_local(const void *va, unsigned >> int flags) >>>>>> else >>>>>> { >>>>>> u32 t = pre_flush(); >>>>>> - unsigned long cr4 = read_cr4(); >>>>>> >>>>>> - write_cr4(cr4 & ~X86_CR4_PGE); >>>>>> - barrier(); >>>>>> - write_cr4(cr4); >>>>>> + if ( !cpu_has_invpcid ) >>>>>> + { >>>>>> + unsigned long cr4 = read_cr4(); >>>>>> + >>>>>> + write_cr4(cr4 & ~X86_CR4_PGE); >>>>>> + barrier(); >>>>>> + write_cr4(cr4); >>>>>> + } >>>>>> + else >>>>>> + { >>>>>> + /* >>>>>> + * Using invpcid to flush all mappings works >>>>>> + * regardless of whether PCID is enabled or not. >>>>>> + * It is faster than read-modify-write CR4. >>>>>> + */ >>>> Its a cr4 double write, rather than RMW. We read from a cached value >>>> anyway, not from hardware. >>>> >>>>>> + invpcid_flush_all(); >>>>>> + } >>>>> The reference to PCID in the comment isn't really meaningful imo. >>>>> PCID and INVPCID are independent features anyway. Also please >>>>> don't create artificially short comment lines. >>>>> >>>>> Generally I also think such if() conditions would better be inverted: >>>>> There's no reason to make the legacy form look as if it was >>>>> preferred. >>>>> >>>>> And then - what about the use in write_cr3() and the two uses that >>>>> remain after my XPTI follow-up series (which sadly looks to be stuck >>>>> for whatever reason), or (without that series) the write_cr3 >>>>> assembler macro? >>>> I don't think it is safe to use invpcid when we're also switching cr3. >>>> The new cr3 may have global pages with different translations, as they >>>> are guest controlled. >>> Can you elaborate a little bit more? >>> >>> How can a guest control any hypervisor mappings? As long as the new cr3 >>> is being loaded before the TLB is flushed via INVPCID I can't see how >>> a problem should occur. >>> >>> In fact my series does exactly what Jan is asking above: it is replacing >>> the remaining cr4 based TLB flushing by INVPCID if possible. So in case >>> there is a flaw in my design please tell me. >> At the moment, we have guest and hypervisor controlled global mappings. >> >> The current switch is: >> cr4 &= ~PGE; >> cr3 = new_cr3; >> cr4 |= PGE; >> >> which means that all global mappings are flushed by the first action, >> and no new global mappings can come into existence. We then switch to >> the new cr3 (again with global fully disabled), then allow global >> mappings to come back into existence. >> >> With the invpcid route, we switch via: >> >> cr3 = new_cr3; >> invpcid all+global; >> >> This has a race window where global mappings are active, and could >> mismatch what is in cr3. This yields #MC on at least some hardware, and >> is specified to have undefined behaviour. > Oh, right, this would be okay only without what used to be named > USER_MAPPINGS_ARE_GLOBAL (and what is now implied). When we start using PCID for user mappings, then we don't need them to be global, at which point we can require/expect that the only global mappings are hypervisor ones which we expect to remain correct across a write to cr3. However, if we do this, then we need to use a bit other than PAGE_GLOBAL to signify guest user mappings. I think this is doable, but I don't think it is going to be trivial to get correct. ~Andrew _______________________________________________ Xen-devel mailing list Xen-devel@lists.xenproject.org https://lists.xenproject.org/mailman/listinfo/xen-devel ^ permalink raw reply [flat|nested] 37+ messages in thread
* Re: [PATCH 2/2] x86: use invpcid to do global flushing 2018-03-05 12:57 ` Andrew Cooper @ 2018-03-05 13:11 ` Juergen Gross 2018-03-05 13:24 ` Jan Beulich 0 siblings, 1 reply; 37+ messages in thread From: Juergen Gross @ 2018-03-05 13:11 UTC (permalink / raw) To: Andrew Cooper, Jan Beulich, Wei Liu; +Cc: Xen-devel On 05/03/18 13:57, Andrew Cooper wrote: > On 05/03/18 12:54, Jan Beulich wrote: >>>>> On 05.03.18 at 13:35, <andrew.cooper3@citrix.com> wrote: >>> On 05/03/18 12:06, Juergen Gross wrote: >>>> On 05/03/18 12:50, Andrew Cooper wrote: >>>>> On 05/03/18 11:31, Jan Beulich wrote: >>>>>>>>> On 05.03.18 at 10:50, <wei.liu2@citrix.com> wrote: >>>>>>> Signed-off-by: Wei Liu <wei.liu2@citrix.com> >>>>>> No description at all? I'd at least expect mention of how much of a >>>>>> performance win this is (for whichever hardware you happen to >>>>>> know that). >>>>>> >>>>>>> @@ -120,11 +121,24 @@ unsigned int flush_area_local(const void *va, unsigned >>> int flags) >>>>>>> else >>>>>>> { >>>>>>> u32 t = pre_flush(); >>>>>>> - unsigned long cr4 = read_cr4(); >>>>>>> >>>>>>> - write_cr4(cr4 & ~X86_CR4_PGE); >>>>>>> - barrier(); >>>>>>> - write_cr4(cr4); >>>>>>> + if ( !cpu_has_invpcid ) >>>>>>> + { >>>>>>> + unsigned long cr4 = read_cr4(); >>>>>>> + >>>>>>> + write_cr4(cr4 & ~X86_CR4_PGE); >>>>>>> + barrier(); >>>>>>> + write_cr4(cr4); >>>>>>> + } >>>>>>> + else >>>>>>> + { >>>>>>> + /* >>>>>>> + * Using invpcid to flush all mappings works >>>>>>> + * regardless of whether PCID is enabled or not. >>>>>>> + * It is faster than read-modify-write CR4. >>>>>>> + */ >>>>> Its a cr4 double write, rather than RMW. We read from a cached value >>>>> anyway, not from hardware. >>>>> >>>>>>> + invpcid_flush_all(); >>>>>>> + } >>>>>> The reference to PCID in the comment isn't really meaningful imo. >>>>>> PCID and INVPCID are independent features anyway. Also please >>>>>> don't create artificially short comment lines. >>>>>> >>>>>> Generally I also think such if() conditions would better be inverted: >>>>>> There's no reason to make the legacy form look as if it was >>>>>> preferred. >>>>>> >>>>>> And then - what about the use in write_cr3() and the two uses that >>>>>> remain after my XPTI follow-up series (which sadly looks to be stuck >>>>>> for whatever reason), or (without that series) the write_cr3 >>>>>> assembler macro? >>>>> I don't think it is safe to use invpcid when we're also switching cr3. >>>>> The new cr3 may have global pages with different translations, as they >>>>> are guest controlled. >>>> Can you elaborate a little bit more? >>>> >>>> How can a guest control any hypervisor mappings? As long as the new cr3 >>>> is being loaded before the TLB is flushed via INVPCID I can't see how >>>> a problem should occur. >>>> >>>> In fact my series does exactly what Jan is asking above: it is replacing >>>> the remaining cr4 based TLB flushing by INVPCID if possible. So in case >>>> there is a flaw in my design please tell me. >>> At the moment, we have guest and hypervisor controlled global mappings. >>> >>> The current switch is: >>> cr4 &= ~PGE; >>> cr3 = new_cr3; >>> cr4 |= PGE; >>> >>> which means that all global mappings are flushed by the first action, >>> and no new global mappings can come into existence. We then switch to >>> the new cr3 (again with global fully disabled), then allow global >>> mappings to come back into existence. >>> >>> With the invpcid route, we switch via: >>> >>> cr3 = new_cr3; >>> invpcid all+global; >>> >>> This has a race window where global mappings are active, and could >>> mismatch what is in cr3. This yields #MC on at least some hardware, and >>> is specified to have undefined behaviour. >> Oh, right, this would be okay only without what used to be named >> USER_MAPPINGS_ARE_GLOBAL (and what is now implied). > > When we start using PCID for user mappings, then we don't need them to > be global, at which point we can require/expect that the only global > mappings are hypervisor ones which we expect to remain correct across a > write to cr3. However, if we do this, then we need to use a bit other > than PAGE_GLOBAL to signify guest user mappings. > > I think this is doable, but I don't think it is going to be trivial to > get correct. Why would we want to keep any global mappings at all? What are they good for? Today the only case I could find where they make sense at all is for 64-bit pv-guests to keep hypervisor mappings in the TLB when the guest is switching between user and kernel mode. In all other cases the complete TLB is flushed when cr3 is loaded with a new value (either due to a Xen context switch or a guest cr3 load being performed by Xen). With using PCID we can just keep cr4.pge being 0 and everything is fine. No additional work required, we can just keep the G bit in the PTEs as a flag which doesn't affect the TLB at all. Juergen _______________________________________________ Xen-devel mailing list Xen-devel@lists.xenproject.org https://lists.xenproject.org/mailman/listinfo/xen-devel ^ permalink raw reply [flat|nested] 37+ messages in thread
* Re: [PATCH 2/2] x86: use invpcid to do global flushing 2018-03-05 13:11 ` Juergen Gross @ 2018-03-05 13:24 ` Jan Beulich 2018-03-05 13:31 ` Wei Liu 0 siblings, 1 reply; 37+ messages in thread From: Jan Beulich @ 2018-03-05 13:24 UTC (permalink / raw) To: Juergen Gross; +Cc: Andrew Cooper, Wei Liu, Xen-devel >>> On 05.03.18 at 14:11, <jgross@suse.com> wrote: > On 05/03/18 13:57, Andrew Cooper wrote: >> When we start using PCID for user mappings, then we don't need them to >> be global, at which point we can require/expect that the only global >> mappings are hypervisor ones which we expect to remain correct across a >> write to cr3. However, if we do this, then we need to use a bit other >> than PAGE_GLOBAL to signify guest user mappings. >> >> I think this is doable, but I don't think it is going to be trivial to >> get correct. > > Why would we want to keep any global mappings at all? What are they good > for? Today the only case I could find where they make sense at all is > for 64-bit pv-guests to keep hypervisor mappings in the TLB when the > guest is switching between user and kernel mode. Hypervisor and guest user mappings, that is. Jan _______________________________________________ Xen-devel mailing list Xen-devel@lists.xenproject.org https://lists.xenproject.org/mailman/listinfo/xen-devel ^ permalink raw reply [flat|nested] 37+ messages in thread
* Re: [PATCH 2/2] x86: use invpcid to do global flushing 2018-03-05 13:24 ` Jan Beulich @ 2018-03-05 13:31 ` Wei Liu 2018-03-05 13:40 ` Andrew Cooper 2018-03-05 13:44 ` Jan Beulich 0 siblings, 2 replies; 37+ messages in thread From: Wei Liu @ 2018-03-05 13:31 UTC (permalink / raw) To: Jan Beulich; +Cc: Juergen Gross, Andrew Cooper, Wei Liu, Xen-devel On Mon, Mar 05, 2018 at 06:24:37AM -0700, Jan Beulich wrote: > >>> On 05.03.18 at 14:11, <jgross@suse.com> wrote: > > On 05/03/18 13:57, Andrew Cooper wrote: > >> When we start using PCID for user mappings, then we don't need them to > >> be global, at which point we can require/expect that the only global > >> mappings are hypervisor ones which we expect to remain correct across a > >> write to cr3. However, if we do this, then we need to use a bit other > >> than PAGE_GLOBAL to signify guest user mappings. > >> > >> I think this is doable, but I don't think it is going to be trivial to > >> get correct. > > > > Why would we want to keep any global mappings at all? What are they good > > for? Today the only case I could find where they make sense at all is > > for 64-bit pv-guests to keep hypervisor mappings in the TLB when the > > guest is switching between user and kernel mode. > > Hypervisor and guest user mappings, that is. > I'm not sure I understand the rationale behind global guest mappings. Is it to keep guest user mappings when switching to guest kernel mode? (A normal kernel like Linux makes global kernel mappings, but obv. we can't do that in 64bit PV guests because kernel is also in ring3.) Wei. _______________________________________________ Xen-devel mailing list Xen-devel@lists.xenproject.org https://lists.xenproject.org/mailman/listinfo/xen-devel ^ permalink raw reply [flat|nested] 37+ messages in thread
* Re: [PATCH 2/2] x86: use invpcid to do global flushing 2018-03-05 13:31 ` Wei Liu @ 2018-03-05 13:40 ` Andrew Cooper 2018-03-05 13:44 ` Jan Beulich 1 sibling, 0 replies; 37+ messages in thread From: Andrew Cooper @ 2018-03-05 13:40 UTC (permalink / raw) To: Wei Liu, Jan Beulich; +Cc: Juergen Gross, Xen-devel On 05/03/18 13:31, Wei Liu wrote: > On Mon, Mar 05, 2018 at 06:24:37AM -0700, Jan Beulich wrote: >>>>> On 05.03.18 at 14:11, <jgross@suse.com> wrote: >>> On 05/03/18 13:57, Andrew Cooper wrote: >>>> When we start using PCID for user mappings, then we don't need them to >>>> be global, at which point we can require/expect that the only global >>>> mappings are hypervisor ones which we expect to remain correct across a >>>> write to cr3. However, if we do this, then we need to use a bit other >>>> than PAGE_GLOBAL to signify guest user mappings. >>>> >>>> I think this is doable, but I don't think it is going to be trivial to >>>> get correct. >>> Why would we want to keep any global mappings at all? What are they good >>> for? Today the only case I could find where they make sense at all is >>> for 64-bit pv-guests to keep hypervisor mappings in the TLB when the >>> guest is switching between user and kernel mode. >> Hypervisor and guest user mappings, that is. >> > I'm not sure I understand the rationale behind global guest mappings. Is > it to keep guest user mappings when switching to guest kernel mode? Yes - it is a performance optimisation so on guest user => guest kernel context switch, guest user mappings are still present in the TLB. ~Andrew _______________________________________________ Xen-devel mailing list Xen-devel@lists.xenproject.org https://lists.xenproject.org/mailman/listinfo/xen-devel ^ permalink raw reply [flat|nested] 37+ messages in thread
* Re: [PATCH 2/2] x86: use invpcid to do global flushing 2018-03-05 13:31 ` Wei Liu 2018-03-05 13:40 ` Andrew Cooper @ 2018-03-05 13:44 ` Jan Beulich 1 sibling, 0 replies; 37+ messages in thread From: Jan Beulich @ 2018-03-05 13:44 UTC (permalink / raw) To: Wei Liu; +Cc: Juergen Gross, Andrew Cooper, Xen-devel >>> On 05.03.18 at 14:31, <wei.liu2@citrix.com> wrote: > On Mon, Mar 05, 2018 at 06:24:37AM -0700, Jan Beulich wrote: >> >>> On 05.03.18 at 14:11, <jgross@suse.com> wrote: >> > On 05/03/18 13:57, Andrew Cooper wrote: >> >> When we start using PCID for user mappings, then we don't need them to >> >> be global, at which point we can require/expect that the only global >> >> mappings are hypervisor ones which we expect to remain correct across a >> >> write to cr3. However, if we do this, then we need to use a bit other >> >> than PAGE_GLOBAL to signify guest user mappings. >> >> >> >> I think this is doable, but I don't think it is going to be trivial to >> >> get correct. >> > >> > Why would we want to keep any global mappings at all? What are they good >> > for? Today the only case I could find where they make sense at all is >> > for 64-bit pv-guests to keep hypervisor mappings in the TLB when the >> > guest is switching between user and kernel mode. >> >> Hypervisor and guest user mappings, that is. >> > > I'm not sure I understand the rationale behind global guest mappings. Is > it to keep guest user mappings when switching to guest kernel mode? Yes, exactly (and especially when - like for some syscalls - there's a fast user->kernel->user round trip). The goal is to avoid at least some of the TLB reloads, when we already can't have global guest kernel mappings. Jan _______________________________________________ Xen-devel mailing list Xen-devel@lists.xenproject.org https://lists.xenproject.org/mailman/listinfo/xen-devel ^ permalink raw reply [flat|nested] 37+ messages in thread
* Re: [PATCH 2/2] x86: use invpcid to do global flushing 2018-03-05 12:35 ` Andrew Cooper 2018-03-05 12:54 ` Jan Beulich @ 2018-03-06 7:10 ` Juergen Gross 1 sibling, 0 replies; 37+ messages in thread From: Juergen Gross @ 2018-03-06 7:10 UTC (permalink / raw) To: Andrew Cooper, Jan Beulich, Wei Liu; +Cc: Xen-devel On 05/03/18 13:35, Andrew Cooper wrote: > On 05/03/18 12:06, Juergen Gross wrote: >> On 05/03/18 12:50, Andrew Cooper wrote: >>> On 05/03/18 11:31, Jan Beulich wrote: >>>>>>> On 05.03.18 at 10:50, <wei.liu2@citrix.com> wrote: >>>>> Signed-off-by: Wei Liu <wei.liu2@citrix.com> >>>> No description at all? I'd at least expect mention of how much of a >>>> performance win this is (for whichever hardware you happen to >>>> know that). >>>> >>>>> @@ -120,11 +121,24 @@ unsigned int flush_area_local(const void *va, unsigned int flags) >>>>> else >>>>> { >>>>> u32 t = pre_flush(); >>>>> - unsigned long cr4 = read_cr4(); >>>>> >>>>> - write_cr4(cr4 & ~X86_CR4_PGE); >>>>> - barrier(); >>>>> - write_cr4(cr4); >>>>> + if ( !cpu_has_invpcid ) >>>>> + { >>>>> + unsigned long cr4 = read_cr4(); >>>>> + >>>>> + write_cr4(cr4 & ~X86_CR4_PGE); >>>>> + barrier(); >>>>> + write_cr4(cr4); >>>>> + } >>>>> + else >>>>> + { >>>>> + /* >>>>> + * Using invpcid to flush all mappings works >>>>> + * regardless of whether PCID is enabled or not. >>>>> + * It is faster than read-modify-write CR4. >>>>> + */ >>> Its a cr4 double write, rather than RMW. We read from a cached value >>> anyway, not from hardware. >>> >>>>> + invpcid_flush_all(); >>>>> + } >>>> The reference to PCID in the comment isn't really meaningful imo. >>>> PCID and INVPCID are independent features anyway. Also please >>>> don't create artificially short comment lines. >>>> >>>> Generally I also think such if() conditions would better be inverted: >>>> There's no reason to make the legacy form look as if it was >>>> preferred. >>>> >>>> And then - what about the use in write_cr3() and the two uses that >>>> remain after my XPTI follow-up series (which sadly looks to be stuck >>>> for whatever reason), or (without that series) the write_cr3 >>>> assembler macro? >>> I don't think it is safe to use invpcid when we're also switching cr3. >>> The new cr3 may have global pages with different translations, as they >>> are guest controlled. >> Can you elaborate a little bit more? >> >> How can a guest control any hypervisor mappings? As long as the new cr3 >> is being loaded before the TLB is flushed via INVPCID I can't see how >> a problem should occur. >> >> In fact my series does exactly what Jan is asking above: it is replacing >> the remaining cr4 based TLB flushing by INVPCID if possible. So in case >> there is a flaw in my design please tell me. > > At the moment, we have guest and hypervisor controlled global mappings. > > The current switch is: > cr4 &= ~PGE; > cr3 = new_cr3; > cr4 |= PGE; > > which means that all global mappings are flushed by the first action, > and no new global mappings can come into existence. We then switch to > the new cr3 (again with global fully disabled), then allow global > mappings to come back into existence. > > With the invpcid route, we switch via: > > cr3 = new_cr3; > invpcid all+global; > > This has a race window where global mappings are active, and could > mismatch what is in cr3. This yields #MC on at least some hardware, and > is specified to have undefined behaviour. Okay, so I will modify my patch series to use INVPCID only when global mappings are disabled. With PCID available I'll use it for the non-XPTI case, too. This will enable us to disable global mappings completely, so INVPCID can always be used for flushing the TLB. Juergen _______________________________________________ Xen-devel mailing list Xen-devel@lists.xenproject.org https://lists.xenproject.org/mailman/listinfo/xen-devel ^ permalink raw reply [flat|nested] 37+ messages in thread
* Re: [PATCH 2/2] x86: use invpcid to do global flushing 2018-03-05 9:50 ` [PATCH 2/2] x86: use invpcid to do global flushing Wei Liu 2018-03-05 9:52 ` Juergen Gross 2018-03-05 11:31 ` Jan Beulich @ 2018-03-09 15:29 ` Jan Beulich [not found] ` <5AA2B67302000078001B0567@suse.com> 3 siblings, 0 replies; 37+ messages in thread From: Jan Beulich @ 2018-03-09 15:29 UTC (permalink / raw) To: Wei Liu; +Cc: Juergen Gross, Andrew Cooper, Xen-devel >>> On 05.03.18 at 10:50, <wei.liu2@citrix.com> wrote: > @@ -120,11 +121,24 @@ unsigned int flush_area_local(const void *va, unsigned int flags) > else > { > u32 t = pre_flush(); > - unsigned long cr4 = read_cr4(); > > - write_cr4(cr4 & ~X86_CR4_PGE); > - barrier(); > - write_cr4(cr4); > + if ( !cpu_has_invpcid ) > + { > + unsigned long cr4 = read_cr4(); > + > + write_cr4(cr4 & ~X86_CR4_PGE); > + barrier(); > + write_cr4(cr4); > + } > + else > + { > + /* > + * Using invpcid to flush all mappings works > + * regardless of whether PCID is enabled or not. > + * It is faster than read-modify-write CR4. > + */ > + invpcid_flush_all(); > + } As just validly indicated by Jürgen, this is where my comment I gave to one of his patches actually belongs: This is correct for FLUSH_TLB_GLOBAL, but goes too far for FLUSH_TLB. Jan _______________________________________________ Xen-devel mailing list Xen-devel@lists.xenproject.org https://lists.xenproject.org/mailman/listinfo/xen-devel ^ permalink raw reply [flat|nested] 37+ messages in thread
[parent not found: <5AA2B67302000078001B0567@suse.com>]
* Re: [PATCH 2/2] x86: use invpcid to do global flushing [not found] ` <5AA2B67302000078001B0567@suse.com> @ 2018-03-09 18:05 ` Juergen Gross 2018-03-12 12:59 ` Jan Beulich 0 siblings, 1 reply; 37+ messages in thread From: Juergen Gross @ 2018-03-09 18:05 UTC (permalink / raw) To: Jan Beulich, Wei Liu; +Cc: Andrew Cooper, Xen-devel On 09/03/18 16:29, Jan Beulich wrote: >>>> On 05.03.18 at 10:50, <wei.liu2@citrix.com> wrote: >> @@ -120,11 +121,24 @@ unsigned int flush_area_local(const void *va, unsigned int flags) >> else >> { >> u32 t = pre_flush(); >> - unsigned long cr4 = read_cr4(); >> >> - write_cr4(cr4 & ~X86_CR4_PGE); >> - barrier(); >> - write_cr4(cr4); >> + if ( !cpu_has_invpcid ) >> + { >> + unsigned long cr4 = read_cr4(); >> + >> + write_cr4(cr4 & ~X86_CR4_PGE); >> + barrier(); >> + write_cr4(cr4); >> + } >> + else >> + { >> + /* >> + * Using invpcid to flush all mappings works >> + * regardless of whether PCID is enabled or not. >> + * It is faster than read-modify-write CR4. >> + */ >> + invpcid_flush_all(); >> + } > > As just validly indicated by Jürgen, this is where my comment I > gave to one of his patches actually belongs: This is correct for > FLUSH_TLB_GLOBAL, but goes too far for FLUSH_TLB. And again it was so even before this patch. Juergen _______________________________________________ Xen-devel mailing list Xen-devel@lists.xenproject.org https://lists.xenproject.org/mailman/listinfo/xen-devel ^ permalink raw reply [flat|nested] 37+ messages in thread
* Re: [PATCH 2/2] x86: use invpcid to do global flushing 2018-03-09 18:05 ` Juergen Gross @ 2018-03-12 12:59 ` Jan Beulich 2018-03-12 13:10 ` Juergen Gross 0 siblings, 1 reply; 37+ messages in thread From: Jan Beulich @ 2018-03-12 12:59 UTC (permalink / raw) To: Juergen Gross; +Cc: andrew.cooper3, wei.liu2, xen-devel >>> Juergen Gross <jgross@suse.com> 03/09/18 7:05 PM >>> >On 09/03/18 16:29, Jan Beulich wrote: >>>>> On 05.03.18 at 10:50, <wei.liu2@citrix.com> wrote: >>> @@ -120,11 +121,24 @@ unsigned int flush_area_local(const void *va, unsigned int flags) >>> else >>> { >>> u32 t = pre_flush(); >>> - unsigned long cr4 = read_cr4(); >>> >>> - write_cr4(cr4 & ~X86_CR4_PGE); >>> - barrier(); >>> - write_cr4(cr4); >>> + if ( !cpu_has_invpcid ) >>> + { >>> + unsigned long cr4 = read_cr4(); >>> + >>> + write_cr4(cr4 & ~X86_CR4_PGE); >>> + barrier(); >>> + write_cr4(cr4); >>> + } >>> + else >>> + { >>> + /* >>> + * Using invpcid to flush all mappings works >>> + * regardless of whether PCID is enabled or not. >>> + * It is faster than read-modify-write CR4. >>> + */ >>> + invpcid_flush_all(); >>> + } >> >> As just validly indicated by Jürgen, this is where my comment I >> gave to one of his patches actually belongs: This is correct for >> FLUSH_TLB_GLOBAL, but goes too far for FLUSH_TLB. > >And again it was so even before this patch. Not exactly - "before this patch" should include the state things were in before 32-bit code got removed. And that's where we had a proper separation between flushes including and excluding global entries. And now that we regain that ability, we should leverage it. Jan _______________________________________________ Xen-devel mailing list Xen-devel@lists.xenproject.org https://lists.xenproject.org/mailman/listinfo/xen-devel ^ permalink raw reply [flat|nested] 37+ messages in thread
* Re: [PATCH 2/2] x86: use invpcid to do global flushing 2018-03-12 12:59 ` Jan Beulich @ 2018-03-12 13:10 ` Juergen Gross 2018-03-12 13:13 ` Jan Beulich ` (3 more replies) 0 siblings, 4 replies; 37+ messages in thread From: Juergen Gross @ 2018-03-12 13:10 UTC (permalink / raw) To: Jan Beulich; +Cc: andrew.cooper3, wei.liu2, xen-devel On 12/03/18 13:59, Jan Beulich wrote: >>>> Juergen Gross <jgross@suse.com> 03/09/18 7:05 PM >>> >> On 09/03/18 16:29, Jan Beulich wrote: >>>>>> On 05.03.18 at 10:50, <wei.liu2@citrix.com> wrote: >>>> @@ -120,11 +121,24 @@ unsigned int flush_area_local(const void *va, unsigned int flags) >>>> else >>>> { >>>> u32 t = pre_flush(); >>>> - unsigned long cr4 = read_cr4(); >>>> >>>> - write_cr4(cr4 & ~X86_CR4_PGE); >>>> - barrier(); >>>> - write_cr4(cr4); >>>> + if ( !cpu_has_invpcid ) >>>> + { >>>> + unsigned long cr4 = read_cr4(); >>>> + >>>> + write_cr4(cr4 & ~X86_CR4_PGE); >>>> + barrier(); >>>> + write_cr4(cr4); >>>> + } >>>> + else >>>> + { >>>> + /* >>>> + * Using invpcid to flush all mappings works >>>> + * regardless of whether PCID is enabled or not. >>>> + * It is faster than read-modify-write CR4. >>>> + */ >>>> + invpcid_flush_all(); >>>> + } >>> >>> As just validly indicated by Jürgen, this is where my comment I >>> gave to one of his patches actually belongs: This is correct for >>> FLUSH_TLB_GLOBAL, but goes too far for FLUSH_TLB. >> >> And again it was so even before this patch. > > Not exactly - "before this patch" should include the state things were in before > 32-bit code got removed. And that's where we had a proper separation between > flushes including and excluding global entries. And now that we regain that > ability, we should leverage it. Already working on it in my XPTI speed-up series. BTW: are you already working on rebasing your XPTI speed up series to current staging? I'd like my series to use your series as a base unless you are telling me you won't be able to resend your series soon. Juergen _______________________________________________ Xen-devel mailing list Xen-devel@lists.xenproject.org https://lists.xenproject.org/mailman/listinfo/xen-devel ^ permalink raw reply [flat|nested] 37+ messages in thread
* Re: [PATCH 2/2] x86: use invpcid to do global flushing 2018-03-12 13:10 ` Juergen Gross @ 2018-03-12 13:13 ` Jan Beulich [not found] ` <5AA67CEF0200007800129C5D@suse.com> ` (2 subsequent siblings) 3 siblings, 0 replies; 37+ messages in thread From: Jan Beulich @ 2018-03-12 13:13 UTC (permalink / raw) To: Juergen Gross; +Cc: andrew.cooper3, wei.liu2, xen-devel >>> Juergen Gross <jgross@suse.com> 03/12/18 2:10 PM >>> >BTW: are you already working on rebasing your XPTI speed up series to >current staging? I'd like my series to use your series as a base unless >you are telling me you won't be able to resend your series soon. I hope to be able to get to this later this week. Jan _______________________________________________ Xen-devel mailing list Xen-devel@lists.xenproject.org https://lists.xenproject.org/mailman/listinfo/xen-devel ^ permalink raw reply [flat|nested] 37+ messages in thread
[parent not found: <5AA67CEF0200007800129C5D@suse.com>]
* Re: [PATCH 2/2] x86: use invpcid to do global flushing [not found] ` <5AA67CEF0200007800129C5D@suse.com> @ 2018-03-12 13:14 ` Juergen Gross 0 siblings, 0 replies; 37+ messages in thread From: Juergen Gross @ 2018-03-12 13:14 UTC (permalink / raw) To: Jan Beulich; +Cc: andrew.cooper3, wei.liu2, xen-devel On 12/03/18 14:13, Jan Beulich wrote: >>>> Juergen Gross <jgross@suse.com> 03/12/18 2:10 PM >>> >> BTW: are you already working on rebasing your XPTI speed up series to >> current staging? I'd like my series to use your series as a base unless >> you are telling me you won't be able to resend your series soon. > > I hope to be able to get to this later this week. Okay, thanks. Juergen _______________________________________________ Xen-devel mailing list Xen-devel@lists.xenproject.org https://lists.xenproject.org/mailman/listinfo/xen-devel ^ permalink raw reply [flat|nested] 37+ messages in thread
* Re: [PATCH 2/2] x86: use invpcid to do global flushing 2018-03-12 13:10 ` Juergen Gross 2018-03-12 13:13 ` Jan Beulich [not found] ` <5AA67CEF0200007800129C5D@suse.com> @ 2018-03-13 8:07 ` Jan Beulich [not found] ` <5AA794B402000078001B0CC7@suse.com> 3 siblings, 0 replies; 37+ messages in thread From: Jan Beulich @ 2018-03-13 8:07 UTC (permalink / raw) To: Juergen Gross; +Cc: andrew.cooper3, wei.liu2, xen-devel >>> On 12.03.18 at 14:10, <jgross@suse.com> wrote: > BTW: are you already working on rebasing your XPTI speed up series to > current staging? I'd like my series to use your series as a base unless > you are telling me you won't be able to resend your series soon. Coming back to my reply here yesterday - then again I'm a little reluctant to send out a new version that has no changes other than being re-based, when there were no comments by Andrew on most of the remaining patches yet (and in the one case where there were comments, I'm afraid I continue to disagree, but I'll see about moving that patch last in the series). Plus staging is broken right now anyway (but of course we know now which patch to revert if need be). Jan _______________________________________________ Xen-devel mailing list Xen-devel@lists.xenproject.org https://lists.xenproject.org/mailman/listinfo/xen-devel ^ permalink raw reply [flat|nested] 37+ messages in thread
[parent not found: <5AA794B402000078001B0CC7@suse.com>]
* Re: [PATCH 2/2] x86: use invpcid to do global flushing [not found] ` <5AA794B402000078001B0CC7@suse.com> @ 2018-03-13 9:27 ` Juergen Gross 2018-03-13 9:35 ` Jan Beulich [not found] ` <5AA7A97702000078001B0D63@suse.com> 0 siblings, 2 replies; 37+ messages in thread From: Juergen Gross @ 2018-03-13 9:27 UTC (permalink / raw) To: Jan Beulich; +Cc: andrew.cooper3, wei.liu2, xen-devel On 13/03/18 09:07, Jan Beulich wrote: >>>> On 12.03.18 at 14:10, <jgross@suse.com> wrote: >> BTW: are you already working on rebasing your XPTI speed up series to >> current staging? I'd like my series to use your series as a base unless >> you are telling me you won't be able to resend your series soon. > > Coming back to my reply here yesterday - then again I'm a little > reluctant to send out a new version that has no changes other > than being re-based, when there were no comments by Andrew > on most of the remaining patches yet (and in the one case > where there were comments, I'm afraid I continue to disagree, > but I'll see about moving that patch last in the series). In order to move forward, would you prefer me to base my patches on current staging and put your patches on top of my series later? I just don't want to rebase my series on a moving target... Juergen _______________________________________________ Xen-devel mailing list Xen-devel@lists.xenproject.org https://lists.xenproject.org/mailman/listinfo/xen-devel ^ permalink raw reply [flat|nested] 37+ messages in thread
* Re: [PATCH 2/2] x86: use invpcid to do global flushing 2018-03-13 9:27 ` Juergen Gross @ 2018-03-13 9:35 ` Jan Beulich [not found] ` <5AA7A97702000078001B0D63@suse.com> 1 sibling, 0 replies; 37+ messages in thread From: Jan Beulich @ 2018-03-13 9:35 UTC (permalink / raw) To: Juergen Gross; +Cc: andrew.cooper3, wei.liu2, xen-devel >>> On 13.03.18 at 10:27, <jgross@suse.com> wrote: > On 13/03/18 09:07, Jan Beulich wrote: >>>>> On 12.03.18 at 14:10, <jgross@suse.com> wrote: >>> BTW: are you already working on rebasing your XPTI speed up series to >>> current staging? I'd like my series to use your series as a base unless >>> you are telling me you won't be able to resend your series soon. >> >> Coming back to my reply here yesterday - then again I'm a little >> reluctant to send out a new version that has no changes other >> than being re-based, when there were no comments by Andrew >> on most of the remaining patches yet (and in the one case >> where there were comments, I'm afraid I continue to disagree, >> but I'll see about moving that patch last in the series). > > In order to move forward, would you prefer me to base my patches on > current staging and put your patches on top of my series later? I just > don't want to rebase my series on a moving target... Well, I'm similarly not intending to re-base onto a series still under development / review. That's really the bad thing with deadlines: If we didn't freeze the tree at a given time, but at the point where previously agreed features and other non-bug-fix changes have landed, we wouldn't have such an ordering problem right now (or to be precise the ordering issue would still be there, but neither of us would be at risk of their changes not making it). Jan _______________________________________________ Xen-devel mailing list Xen-devel@lists.xenproject.org https://lists.xenproject.org/mailman/listinfo/xen-devel ^ permalink raw reply [flat|nested] 37+ messages in thread
[parent not found: <5AA7A97702000078001B0D63@suse.com>]
* Re: [PATCH 2/2] x86: use invpcid to do global flushing [not found] ` <5AA7A97702000078001B0D63@suse.com> @ 2018-03-13 9:48 ` Juergen Gross 2018-03-13 10:22 ` Jan Beulich 0 siblings, 1 reply; 37+ messages in thread From: Juergen Gross @ 2018-03-13 9:48 UTC (permalink / raw) To: Jan Beulich, andrew.cooper3; +Cc: xen-devel, wei.liu2 On 13/03/18 10:35, Jan Beulich wrote: >>>> On 13.03.18 at 10:27, <jgross@suse.com> wrote: >> On 13/03/18 09:07, Jan Beulich wrote: >>>>>> On 12.03.18 at 14:10, <jgross@suse.com> wrote: >>>> BTW: are you already working on rebasing your XPTI speed up series to >>>> current staging? I'd like my series to use your series as a base unless >>>> you are telling me you won't be able to resend your series soon. >>> >>> Coming back to my reply here yesterday - then again I'm a little >>> reluctant to send out a new version that has no changes other >>> than being re-based, when there were no comments by Andrew >>> on most of the remaining patches yet (and in the one case >>> where there were comments, I'm afraid I continue to disagree, >>> but I'll see about moving that patch last in the series). >> >> In order to move forward, would you prefer me to base my patches on >> current staging and put your patches on top of my series later? I just >> don't want to rebase my series on a moving target... > > Well, I'm similarly not intending to re-base onto a series still under > development / review. That's really the bad thing with deadlines: > If we didn't freeze the tree at a given time, but at the point where > previously agreed features and other non-bug-fix changes have > landed, we wouldn't have such an ordering problem right now (or > to be precise the ordering issue would still be there, but neither > of us would be at risk of their changes not making it). Understandable. Could you then please repost at least patch 3? It has been approved by Andrew and just needs the formal R-b: after rebasing. Andrew, do you think you can find some time commenting on Jan's other patches of his Meltdown series? Speaking with my release manager hat on I'd _really_ like to have all Meltdown/Spectre related patches minimizing the performance penalties in 4.11. Juergen _______________________________________________ Xen-devel mailing list Xen-devel@lists.xenproject.org https://lists.xenproject.org/mailman/listinfo/xen-devel ^ permalink raw reply [flat|nested] 37+ messages in thread
* Re: [PATCH 2/2] x86: use invpcid to do global flushing 2018-03-13 9:48 ` Juergen Gross @ 2018-03-13 10:22 ` Jan Beulich 2018-03-13 10:26 ` Andrew Cooper 0 siblings, 1 reply; 37+ messages in thread From: Jan Beulich @ 2018-03-13 10:22 UTC (permalink / raw) To: Juergen Gross; +Cc: andrew.cooper3, wei.liu2, xen-devel >>> On 13.03.18 at 10:48, <jgross@suse.com> wrote: > On 13/03/18 10:35, Jan Beulich wrote: >>>>> On 13.03.18 at 10:27, <jgross@suse.com> wrote: >>> On 13/03/18 09:07, Jan Beulich wrote: >>>>>>> On 12.03.18 at 14:10, <jgross@suse.com> wrote: >>>>> BTW: are you already working on rebasing your XPTI speed up series to >>>>> current staging? I'd like my series to use your series as a base unless >>>>> you are telling me you won't be able to resend your series soon. >>>> >>>> Coming back to my reply here yesterday - then again I'm a little >>>> reluctant to send out a new version that has no changes other >>>> than being re-based, when there were no comments by Andrew >>>> on most of the remaining patches yet (and in the one case >>>> where there were comments, I'm afraid I continue to disagree, >>>> but I'll see about moving that patch last in the series). >>> >>> In order to move forward, would you prefer me to base my patches on >>> current staging and put your patches on top of my series later? I just >>> don't want to rebase my series on a moving target... >> >> Well, I'm similarly not intending to re-base onto a series still under >> development / review. That's really the bad thing with deadlines: >> If we didn't freeze the tree at a given time, but at the point where >> previously agreed features and other non-bug-fix changes have >> landed, we wouldn't have such an ordering problem right now (or >> to be precise the ordering issue would still be there, but neither >> of us would be at risk of their changes not making it). > > Understandable. > > Could you then please repost at least patch 3? It has been approved > by Andrew and just needs the formal R-b: after rebasing. Well, the thing with that patch is that from what I've been able to tell so far its re-basing will consist of dropping it, moving its sole remaining hunk (the altinstruction_nop assembler macro) into what has been patch 4. None of the uses of the macro in patch 3 should be needed anymore after Andrew's changes. Or actually I think the macro won't be needed as a standalone one anymore at all, as there's only a single place where it's used, and hence it would likely better be folded into there (the ALTERNATIVE_NOP one). Jan _______________________________________________ Xen-devel mailing list Xen-devel@lists.xenproject.org https://lists.xenproject.org/mailman/listinfo/xen-devel ^ permalink raw reply [flat|nested] 37+ messages in thread
* Re: [PATCH 2/2] x86: use invpcid to do global flushing 2018-03-13 10:22 ` Jan Beulich @ 2018-03-13 10:26 ` Andrew Cooper 0 siblings, 0 replies; 37+ messages in thread From: Andrew Cooper @ 2018-03-13 10:26 UTC (permalink / raw) To: Jan Beulich, Juergen Gross; +Cc: xen-devel, wei.liu2 On 13/03/2018 10:22, Jan Beulich wrote: >>>> On 13.03.18 at 10:48, <jgross@suse.com> wrote: >> On 13/03/18 10:35, Jan Beulich wrote: >>>>>> On 13.03.18 at 10:27, <jgross@suse.com> wrote: >>>> On 13/03/18 09:07, Jan Beulich wrote: >>>>>>>> On 12.03.18 at 14:10, <jgross@suse.com> wrote: >>>>>> BTW: are you already working on rebasing your XPTI speed up series to >>>>>> current staging? I'd like my series to use your series as a base unless >>>>>> you are telling me you won't be able to resend your series soon. >>>>> Coming back to my reply here yesterday - then again I'm a little >>>>> reluctant to send out a new version that has no changes other >>>>> than being re-based, when there were no comments by Andrew >>>>> on most of the remaining patches yet (and in the one case >>>>> where there were comments, I'm afraid I continue to disagree, >>>>> but I'll see about moving that patch last in the series). >>>> In order to move forward, would you prefer me to base my patches on >>>> current staging and put your patches on top of my series later? I just >>>> don't want to rebase my series on a moving target... >>> Well, I'm similarly not intending to re-base onto a series still under >>> development / review. That's really the bad thing with deadlines: >>> If we didn't freeze the tree at a given time, but at the point where >>> previously agreed features and other non-bug-fix changes have >>> landed, we wouldn't have such an ordering problem right now (or >>> to be precise the ordering issue would still be there, but neither >>> of us would be at risk of their changes not making it). >> Understandable. >> >> Could you then please repost at least patch 3? It has been approved >> by Andrew and just needs the formal R-b: after rebasing. > Well, the thing with that patch is that from what I've been able > to tell so far its re-basing will consist of dropping it, moving its > sole remaining hunk (the altinstruction_nop assembler macro) > into what has been patch 4. None of the uses of the macro in > patch 3 should be needed anymore after Andrew's changes. > Or actually I think the macro won't be needed as a standalone > one anymore at all, as there's only a single place where it's > used, and hence it would likely better be folded into there (the > ALTERNATIVE_NOP one). I'm sorry - I realise I'm very behind on reviews atm. If you've got the series rebased, then it will be easier to review this version than the old version. ~Andrew _______________________________________________ Xen-devel mailing list Xen-devel@lists.xenproject.org https://lists.xenproject.org/mailman/listinfo/xen-devel ^ permalink raw reply [flat|nested] 37+ messages in thread
end of thread, other threads:[~2018-03-13 10:26 UTC | newest] Thread overview: 37+ messages (download: mbox.gz / follow: Atom feed) -- links below jump to the message on this page -- 2018-03-05 9:50 [PATCH 0/2] Use invpcid Wei Liu 2018-03-05 9:50 ` [PATCH 1/2] x86: report if PCID and INVPCID are supported Wei Liu 2018-03-05 9:51 ` Juergen Gross 2018-03-05 11:20 ` Jan Beulich 2018-03-05 11:31 ` Wei Liu 2018-03-05 11:48 ` Andrew Cooper [not found] ` <5A9D362002000078001AE74F@suse.com> 2018-03-05 11:43 ` Juergen Gross 2018-03-05 12:37 ` Jan Beulich [not found] ` <5A9D482202000078001AE7ED@suse.com> 2018-03-05 12:49 ` Juergen Gross 2018-03-05 12:57 ` Jan Beulich 2018-03-05 13:26 ` Wei Liu 2018-03-05 9:50 ` [PATCH 2/2] x86: use invpcid to do global flushing Wei Liu 2018-03-05 9:52 ` Juergen Gross 2018-03-05 11:31 ` Jan Beulich 2018-03-05 11:50 ` Andrew Cooper 2018-03-05 12:06 ` Juergen Gross 2018-03-05 12:35 ` Andrew Cooper 2018-03-05 12:54 ` Jan Beulich 2018-03-05 12:57 ` Andrew Cooper 2018-03-05 13:11 ` Juergen Gross 2018-03-05 13:24 ` Jan Beulich 2018-03-05 13:31 ` Wei Liu 2018-03-05 13:40 ` Andrew Cooper 2018-03-05 13:44 ` Jan Beulich 2018-03-06 7:10 ` Juergen Gross 2018-03-09 15:29 ` Jan Beulich [not found] ` <5AA2B67302000078001B0567@suse.com> 2018-03-09 18:05 ` Juergen Gross 2018-03-12 12:59 ` Jan Beulich 2018-03-12 13:10 ` Juergen Gross 2018-03-12 13:13 ` Jan Beulich [not found] ` <5AA67CEF0200007800129C5D@suse.com> 2018-03-12 13:14 ` Juergen Gross 2018-03-13 8:07 ` Jan Beulich [not found] ` <5AA794B402000078001B0CC7@suse.com> 2018-03-13 9:27 ` Juergen Gross 2018-03-13 9:35 ` Jan Beulich [not found] ` <5AA7A97702000078001B0D63@suse.com> 2018-03-13 9:48 ` Juergen Gross 2018-03-13 10:22 ` Jan Beulich 2018-03-13 10:26 ` Andrew Cooper
This is an external index of several public inboxes, see mirroring instructions on how to clone and mirror all data and code used by this external index.