| Message ID | 573B067302000078000EC093@prv-mh.provo.novell.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
On 17/05/16 10:54, Jan Beulich wrote: > Instead of just latching cr4_pv32_mask into %rdx, correct the found > wrong value in %cr4 (to avoid triggering another BUG). The value left > in %rdx should be sufficient for deducing cr4_pv32_mask from the > register dump. Alternatively, you can reuse %rax (as its value is useless by this point) and leave %rdx as exactly cr4_pv32_mask. This avoids needing a subsequent step to reverse engineer cr4_pv32_mask. > > Also there is one more place for XEN_CR4_PV32_BITS to be used. So there is. Sorry for missing it. ~Andrew > > Signed-off-by: Jan Beulich <jbeulich@suse.com> > > --- a/xen/arch/x86/x86_64/compat/entry.S > +++ b/xen/arch/x86/x86_64/compat/entry.S > @@ -182,7 +182,7 @@ ENTRY(compat_restore_all_guest) > testb $3,UREGS_cs(%rsp) > jpe .Lcr4_alt_end > mov CPUINFO_cr4-CPUINFO_guest_cpu_user_regs(%rsp), %rax > - and $~(X86_CR4_SMEP|X86_CR4_SMAP), %rax > + and $~XEN_CR4_PV32_BITS, %rax > mov %rax, CPUINFO_cr4-CPUINFO_guest_cpu_user_regs(%rsp) > mov %rax, %cr4 > .Lcr4_alt_end: > @@ -218,8 +218,10 @@ ENTRY(cr4_pv32_restore) > and cr4_pv32_mask(%rip), %rax > cmp cr4_pv32_mask(%rip), %rax > je 1f > - /* Cause cr4_pv32_mask to be visible in the BUG register dump. */ > - mov cr4_pv32_mask(%rip), %rdx > + /* Avoid coming back here while handling the #UD we cause below. */ > + mov %cr4, %rdx > + or cr4_pv32_mask(%rip), %rdx > + mov %rdx, %cr4 > BUG > 1: > #endif > > > > > > _______________________________________________ > Xen-devel mailing list > Xen-devel@lists.xen.org > http://lists.xen.org/xen-devel
>>> On 17.05.16 at 12:28, <andrew.cooper3@citrix.com> wrote: > On 17/05/16 10:54, Jan Beulich wrote: >> Instead of just latching cr4_pv32_mask into %rdx, correct the found >> wrong value in %cr4 (to avoid triggering another BUG). The value left >> in %rdx should be sufficient for deducing cr4_pv32_mask from the >> register dump. > > Alternatively, you can reuse %rax (as its value is useless by this > point) and leave %rdx as exactly cr4_pv32_mask. This avoids needing a > subsequent step to reverse engineer cr4_pv32_mask. I don't view the value in %rax as useless - that's the set of bits we have found set, which didn't match our expectation. Hence I specifically don't want to re-use that register. Jan
On 17/05/16 11:42, Jan Beulich wrote: >>>> On 17.05.16 at 12:28, <andrew.cooper3@citrix.com> wrote: >> On 17/05/16 10:54, Jan Beulich wrote: >>> Instead of just latching cr4_pv32_mask into %rdx, correct the found >>> wrong value in %cr4 (to avoid triggering another BUG). The value left >>> in %rdx should be sufficient for deducing cr4_pv32_mask from the >>> register dump. >> Alternatively, you can reuse %rax (as its value is useless by this >> point) and leave %rdx as exactly cr4_pv32_mask. This avoids needing a >> subsequent step to reverse engineer cr4_pv32_mask. > I don't view the value in %rax as useless - that's the set of bits > we have found set, which didn't match our expectation. Hence I > specifically don't want to re-use that register. Ok. We don't need to preserve any registers from the caller, so using a different one such as %rbx or %rcx would be fine. The issue is that it is important to see precisely what cr4_pv32_mask is. ~Andrew
>>> On 17.05.16 at 14:56, <andrew.cooper3@citrix.com> wrote: > On 17/05/16 11:42, Jan Beulich wrote: >>>>> On 17.05.16 at 12:28, <andrew.cooper3@citrix.com> wrote: >>> On 17/05/16 10:54, Jan Beulich wrote: >>>> Instead of just latching cr4_pv32_mask into %rdx, correct the found >>>> wrong value in %cr4 (to avoid triggering another BUG). The value left >>>> in %rdx should be sufficient for deducing cr4_pv32_mask from the >>>> register dump. >>> Alternatively, you can reuse %rax (as its value is useless by this >>> point) and leave %rdx as exactly cr4_pv32_mask. This avoids needing a >>> subsequent step to reverse engineer cr4_pv32_mask. >> I don't view the value in %rax as useless - that's the set of bits >> we have found set, which didn't match our expectation. Hence I >> specifically don't want to re-use that register. > > Ok. We don't need to preserve any registers from the caller, so using a > different one such as %rbx or %rcx would be fine. Well, I can certainly (yet hesitantly) do that (and I see no harm in clobbering another register), but ... > The issue is that it is important to see precisely what cr4_pv32_mask is. ... it's not really clear to me what case you see where the value just written to CR4 won't give you all the information you need. After all cr4_pv32_mask doesn't have an awful lot of possible values. Jan
--- a/xen/arch/x86/x86_64/compat/entry.S +++ b/xen/arch/x86/x86_64/compat/entry.S @@ -182,7 +182,7 @@ ENTRY(compat_restore_all_guest) testb $3,UREGS_cs(%rsp) jpe .Lcr4_alt_end mov CPUINFO_cr4-CPUINFO_guest_cpu_user_regs(%rsp), %rax - and $~(X86_CR4_SMEP|X86_CR4_SMAP), %rax + and $~XEN_CR4_PV32_BITS, %rax mov %rax, CPUINFO_cr4-CPUINFO_guest_cpu_user_regs(%rsp) mov %rax, %cr4 .Lcr4_alt_end: @@ -218,8 +218,10 @@ ENTRY(cr4_pv32_restore) and cr4_pv32_mask(%rip), %rax cmp cr4_pv32_mask(%rip), %rax je 1f - /* Cause cr4_pv32_mask to be visible in the BUG register dump. */ - mov cr4_pv32_mask(%rip), %rdx + /* Avoid coming back here while handling the #UD we cause below. */ + mov %cr4, %rdx + or cr4_pv32_mask(%rip), %rdx + mov %rdx, %cr4 BUG 1: #endif