| Message ID | 1503316530-28107-1-git-send-email-andrew.cooper3@citrix.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
>>> On 21.08.17 at 13:55, <andrew.cooper3@citrix.com> wrote: > This avoids unnecessary updates to the stack shadow copy of cr4 during > critical regions with interrupts disabled. Hmm, yes - we don't access CR4 in #MC or NMI handling, do we? > No change in behaviour. > > Signed-off-by: Andrew Cooper <andrew.cooper3@citrix.com> Reviewed-by: Jan Beulich <jbeulich@suse.com> with ... > --- a/xen/arch/x86/flushtlb.c > +++ b/xen/arch/x86/flushtlb.c > @@ -81,9 +81,14 @@ void write_cr3(unsigned long cr3) > > hvm_flush_guest_tlbs(); > > - write_cr4(cr4 & ~X86_CR4_PGE); > - asm volatile ( "mov %0, %%cr3" : : "r" (cr3) : "memory" ); > - write_cr4(cr4); > + asm volatile ("mov %[npge], %%cr4;" > + "mov %[cr3], %%cr3;" > + "mov %[cr4], %%cr4;" > + :: > + [npge] "r" (cr4 & ~X86_CR4_PGE), > + [cr3] "r" (cr3), > + [cr4] "r" (cr4) > + : "memory"); ... blanks added immediately inside the parentheses here and ... > @@ -123,9 +128,11 @@ unsigned int flush_area_local(const void *va, unsigned int flags) > > hvm_flush_guest_tlbs(); > > - write_cr4(cr4 & ~X86_CR4_PGE); > - barrier(); > - write_cr4(cr4); > + asm volatile ("mov %[npge], %%cr4;" > + "mov %[cr4], %%cr4;" > + :: > + [npge] "r" (cr4 & ~X86_CR4_PGE), > + [cr4] "r" (cr4)); ... here. Jan
On 21/08/17 13:46, Jan Beulich wrote: >>>> On 21.08.17 at 13:55, <andrew.cooper3@citrix.com> wrote: >> This avoids unnecessary updates to the stack shadow copy of cr4 during >> critical regions with interrupts disabled. > Hmm, yes - we don't access CR4 in #MC or NMI handling, do we? #MC and NMI are always able to observe stale values whether we update the shadow copy here or not. The entry paths do modify cr4 to re-enable cr4_pv32_mask, but that is all. As we are interrupting Xen context here, we should take the fastpath and leave cr4 unmodified. ~Andrew > >> No change in behaviour. >> >> Signed-off-by: Andrew Cooper <andrew.cooper3@citrix.com> > Reviewed-by: Jan Beulich <jbeulich@suse.com> > with ... > >> --- a/xen/arch/x86/flushtlb.c >> +++ b/xen/arch/x86/flushtlb.c >> @@ -81,9 +81,14 @@ void write_cr3(unsigned long cr3) >> >> hvm_flush_guest_tlbs(); >> >> - write_cr4(cr4 & ~X86_CR4_PGE); >> - asm volatile ( "mov %0, %%cr3" : : "r" (cr3) : "memory" ); >> - write_cr4(cr4); >> + asm volatile ("mov %[npge], %%cr4;" >> + "mov %[cr3], %%cr3;" >> + "mov %[cr4], %%cr4;" >> + :: >> + [npge] "r" (cr4 & ~X86_CR4_PGE), >> + [cr3] "r" (cr3), >> + [cr4] "r" (cr4) >> + : "memory"); > ... blanks added immediately inside the parentheses here and ... > >> @@ -123,9 +128,11 @@ unsigned int flush_area_local(const void *va, unsigned int flags) >> >> hvm_flush_guest_tlbs(); >> >> - write_cr4(cr4 & ~X86_CR4_PGE); >> - barrier(); >> - write_cr4(cr4); >> + asm volatile ("mov %[npge], %%cr4;" >> + "mov %[cr4], %%cr4;" >> + :: >> + [npge] "r" (cr4 & ~X86_CR4_PGE), >> + [cr4] "r" (cr4)); > ... here. > > Jan >
diff --git a/xen/arch/x86/flushtlb.c b/xen/arch/x86/flushtlb.c index f6d7ad1..65012c9 100644 --- a/xen/arch/x86/flushtlb.c +++ b/xen/arch/x86/flushtlb.c @@ -81,9 +81,14 @@ void write_cr3(unsigned long cr3) hvm_flush_guest_tlbs(); - write_cr4(cr4 & ~X86_CR4_PGE); - asm volatile ( "mov %0, %%cr3" : : "r" (cr3) : "memory" ); - write_cr4(cr4); + asm volatile ("mov %[npge], %%cr4;" + "mov %[cr3], %%cr3;" + "mov %[cr4], %%cr4;" + :: + [npge] "r" (cr4 & ~X86_CR4_PGE), + [cr3] "r" (cr3), + [cr4] "r" (cr4) + : "memory"); post_flush(t); @@ -123,9 +128,11 @@ unsigned int flush_area_local(const void *va, unsigned int flags) hvm_flush_guest_tlbs(); - write_cr4(cr4 & ~X86_CR4_PGE); - barrier(); - write_cr4(cr4); + asm volatile ("mov %[npge], %%cr4;" + "mov %[cr4], %%cr4;" + :: + [npge] "r" (cr4 & ~X86_CR4_PGE), + [cr4] "r" (cr4)); post_flush(t); }
This avoids unnecessary updates to the stack shadow copy of cr4 during critical regions with interrupts disabled. No change in behaviour. Signed-off-by: Andrew Cooper <andrew.cooper3@citrix.com> --- CC: Jan Beulich <JBeulich@suse.com> --- xen/arch/x86/flushtlb.c | 19 +++++++++++++------ 1 file changed, 13 insertions(+), 6 deletions(-)