xen-devel.lists.xenproject.org archive mirror
 help / color / mirror / Atom feed
From: Konrad Rzeszutek Wilk <konrad.wilk@oracle.com>
To: Jan Beulich <JBeulich@suse.com>
Cc: weidong.han@intel.com, Jeremy Fitzhardinge <jeremy@goop.org>,
	Tim.Deegan@citrix.com, xen-devel <xen-devel@lists.xen.org>
Subject: Re: xsave=0 workaround needed on 3.2 kernels with Xen 4.1 or Xen-unstable.
Date: Mon, 7 May 2012 12:07:29 -0400	[thread overview]
Message-ID: <20120507160729.GA30800@phenom.dumpdata.com> (raw)
In-Reply-To: <4FA792C00200007800081F06@nat28.tlf.novell.com>

On Mon, May 07, 2012 at 08:15:44AM +0100, Jan Beulich wrote:
> >>> On 04.05.12 at 21:30, AP <apxeng@gmail.com> wrote:
> > From the above I realized that X86_CR4_OSXSAVE was never getting set
> > in v->arch.pv_vcpu.ctrlreg[4].
> 
> Yes, that was the observation in the previous thread too, but the
> reporter didn't seem interested in continuing on from there.
> 
> 
> > So I tried the following patch:
> >
> > diff -r 5a0d60bb536b xen/arch/x86/domain.c
> > --- a/xen/arch/x86/domain.c	Fri Apr 27 21:10:59 2012 -0700
> > +++ b/xen/arch/x86/domain.c	Fri May 04 12:23:57 2012 -0700
> > @@ -691,8 +691,6 @@ unsigned long pv_guest_cr4_fixup(const s
> >          hv_cr4_mask &= ~X86_CR4_DE;
> >      if ( cpu_has_fsgsbase && !is_pv_32bit_domain(v->domain) )
> >          hv_cr4_mask &= ~X86_CR4_FSGSBASE;
> > -    if ( xsave_enabled(v) )
> > -        hv_cr4_mask &= ~X86_CR4_OSXSAVE;
> > 
> >      if ( (guest_cr4 & hv_cr4_mask) != (hv_cr4 & hv_cr4_mask) )
> >          gdprintk(XENLOG_WARNING,
> > diff -r 5a0d60bb536b xen/include/asm-x86/domain.h
> > --- a/xen/include/asm-x86/domain.h	Fri Apr 27 21:10:59 2012 -0700
> > +++ b/xen/include/asm-x86/domain.h	Fri May 04 12:23:57 2012 -0700
> > @@ -530,7 +530,7 @@ unsigned long pv_guest_cr4_fixup(const s
> >       & ~X86_CR4_DE)
> >  #define real_cr4_to_pv_guest_cr4(c)                         \
> >      ((c) & ~(X86_CR4_PGE | X86_CR4_PSE | X86_CR4_TSD        \
> > -             | X86_CR4_OSXSAVE | X86_CR4_SMEP))
> > +             | X86_CR4_SMEP))
> > 
> >  void domain_cpuid(struct domain *d,
> >                    unsigned int  input,
> 
> No, this is specifically the wrong thing. From what we know so far
> (i.e. the outcome of the above printing you added) the problem in
> in the Dom0 kernel (in it never setting CR4.OSXSAVE prior to
> attempting XSETBV). What your patch efectively does is take away
> control from the guest kernels to control the (virtual) CR4 flag...
> 
> > That allowed the system to boot successfully though I did see the
> > following message:
> > (XEN) domain.c:698:d0 Attempt to change CR4 flags 00042660 -> 00002660
> 
> ... which is what this message is telling you.
> 
> > Not sure if the above patch is right fix but I hope it was at least
> > helpful in pointing at where the problem might be.
> > 
> > BTW, I see the same invalid op issue with Xen 4.1.2 if I boot with xsave=1.
> 
> Sure, as it's a kernel problem. It's the kernel that needs logging added,
> to find out why the CR4 write supposedly happening immediately
> prior to the XSETBV (set_in_cr4(X86_CR4_OSXSAVE)) doesn't actually
> happen, or doesn't set the flag. Perhaps something fishy going on
> with the paravirt ops patching, since the disassembly of the opcode
> bytes shown with the oops message are indicating that the right
> thing is being attempted:
> 
> ff 14 25 10 33 c1 81 	callq  *0xffffffff81c13310 [so xen_read_cr4 which is native_read_cr4]
> 48 89 c7             	mov    %rax,%rdi
> 48 81 cf 00 00 04 00 	or     $0x40000,%rdi
>                      	       ^^^^^^^^
> ff 14 25 18 33 c1 81 	callq  *0xffffffff81c13318 [so xen_write_cr4] - which is filtering X86_CR4_PGE and X86_CR4_PSE]
> 48 8b 05 0d 15 db 00 	mov    0xdb150d(%rip),%rax
> 31 c9                	xor    %ecx,%ecx
> 48 89 c2             	mov    %rax,%rdx
> 48 c1 ea 20          	shr    $0x20,%rdx
> 0f 01 d1             	xsetbv 
> 5d                   	pop    %rbp
> c3                   	retq   
> 
> The primary thing that strikes me as odd is that both calls are still
> indirect ones, even though I thought that they should get replaced
> by direct ones (or even the actual instruction, namely in the
> read_cr4() case) upon first use. Konrad, Jeremy - am I wrong here?

They do get replaced (during runtime - and this is done by the
alternative_instructions). Is this output from objdump or straight
from the memory (so using the Xen debugger?).


> 
> And the dumped %rdi value indicates that bit 18 did _not_ get set.

That would imply that xen_write_cr4, which is just mov to cr4
is getting trapped but somehow the hypervisor isn't setting the
rdi value? Or maybe the the native_write_cr4 ends up
filtering in the wrong order?

   0xffffffff8102e650 <xen_write_cr4>:  push   %rbp
   0xffffffff8102e651 <xen_write_cr4+1>:        and    $0x6f,%dil
   0xffffffff8102e655 <xen_write_cr4+5>:        mov    %rsp,%rbp
   0xffffffff8102e658 <xen_write_cr4+8>:        mov    %rdi,%cr4
   0xffffffff8102e65b <xen_write_cr4+11>:       leaveq
   0xffffffff8102e65c <xen_write_cr4+12>:       retq


> 
> Jan

  reply	other threads:[~2012-05-07 16:07 UTC|newest]

Thread overview: 26+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2012-04-30 19:37 xsave=0 workaround needed on 3.2 kernels with Xen 4.1 or Xen-unstable Konrad Rzeszutek Wilk
2012-05-02  9:00 ` Jan Beulich
2012-05-02 18:42   ` AP
2012-05-03  9:15     ` Jan Beulich
2012-05-03 18:09       ` AP
2012-05-04 19:30         ` AP
2012-05-07  7:15           ` Jan Beulich
2012-05-07 16:07             ` Konrad Rzeszutek Wilk [this message]
2012-05-07 22:55             ` Jeremy Fitzhardinge
2012-05-07 23:57             ` AP
2012-05-08  0:08               ` Konrad Rzeszutek Wilk
2012-05-08  0:41                 ` AP
2012-05-08 16:39                   ` Konrad Rzeszutek Wilk
2012-05-08 17:02                     ` Matt Wilson
2012-05-09  0:35                   ` Konrad Rzeszutek Wilk
2012-05-09 13:11                     ` Konrad Rzeszutek Wilk
2012-05-09 13:30                       ` Ian Campbell
2012-05-09 13:34                       ` Jan Beulich
2012-05-09 17:38                         ` Konrad Rzeszutek Wilk
2012-05-10 19:39                           ` Jeff Law
2012-05-10 20:57                             ` Konrad Rzeszutek Wilk
2012-05-11  0:58                               ` Konrad Rzeszutek Wilk
2012-05-11  2:27                                 ` Jeff Law
2012-05-11  8:23                             ` Jan Beulich
2012-05-10 20:15                           ` Jeff Law
2012-05-08  6:25               ` Jan Beulich

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20120507160729.GA30800@phenom.dumpdata.com \
    --to=konrad.wilk@oracle.com \
    --cc=JBeulich@suse.com \
    --cc=Tim.Deegan@citrix.com \
    --cc=jeremy@goop.org \
    --cc=weidong.han@intel.com \
    --cc=xen-devel@lists.xen.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).