>>> On 13.02.18 at 11:07, <andrew.coop...@citrix.com> wrote: > On 13/02/2018 09:56, Jan Beulich wrote: >>>>> On 12.02.18 at 13:30, <wei.l...@citrix.com> wrote: >>> On Mon, Feb 12, 2018 at 11:23:04AM +0000, Andrew Cooper wrote: >>>> diff --git a/xen/arch/x86/x86_64/entry.S b/xen/arch/x86/x86_64/entry.S >>>> index 58f652d..bd3819a 100644 >>>> --- a/xen/arch/x86/x86_64/entry.S >>>> +++ b/xen/arch/x86/x86_64/entry.S >>>> @@ -557,23 +557,9 @@ handle_exception_saved: >>>> testb $X86_EFLAGS_IF>>8,UREGS_eflags+1(%rsp) >>>> jz exception_with_ints_disabled >>>> >>>> -.Lcr4_pv32_orig: >>>> - jmp .Lcr4_pv32_done >>>> - .skip (.Lcr4_pv32_alt_end - .Lcr4_pv32_alt) - (. - >>>> .Lcr4_pv32_orig), 0xcc >>>> - .pushsection .altinstr_replacement, "ax" >>>> -.Lcr4_pv32_alt: >>>> - mov VCPU_domain(%rbx),%rax >>>> -.Lcr4_pv32_alt_end: >>>> - .section .altinstructions, "a" >>>> - altinstruction_entry .Lcr4_pv32_orig, .Lcr4_pv32_alt, \ >>>> - X86_FEATURE_XEN_SMEP, \ >>>> - (.Lcr4_pv32_alt_end - .Lcr4_pv32_alt), \ >>>> - (.Lcr4_pv32_alt_end - .Lcr4_pv32_alt) >>>> - altinstruction_entry .Lcr4_pv32_orig, .Lcr4_pv32_alt, \ >>>> - X86_FEATURE_XEN_SMAP, \ >>>> - (.Lcr4_pv32_alt_end - .Lcr4_pv32_alt), \ >>>> - (.Lcr4_pv32_alt_end - .Lcr4_pv32_alt) >>>> - .popsection >>>> + ALTERNATIVE_2 "jmp .Lcr4_pv32_done; .skip 2, 0x90", \ >>> This changed 0xcc to 0x90 but since it is just padding following an >>> unconditional jmp so it shouldn't matter. >> Well, it was for that very reason that I had picked 0xcc originally: >> We better know if some branch mistakenly leads into that region. > > Know how? At the time you wrote this, Xen silently executed its way > through debug traps, and it took some persuading to get you to ok the > patch which at least printed a line every time we a breakpoint in > hypervisor space.
Granted I didn't realize at the time that breakpoints would go all silent. > If you actually want to notice going down the wrong path, then you want > a BUG. I'd be very much in favor of this, if only there was a single byte insn documented to cause #UD now and forever. Abusing what is INTO or SALC outside of 64-bit mode doesn't look very attractive. >> I also very much object to the literal 2 passed as an argument to >> .skip above: What if the label moves out far enough that a short >> branch won't be usable anymore? > > Is the commit message not enough? a) its not going to change, because > it hasn't changed since you put the code in originally and I don't > expect it to in the future, and b) it is a temporary necessary > requirement to make the series bisectable and reviewable. This skip is > dropped in patch 6 when the automatic padding calculations work. Oh, if it goes away by the end of the series, then that's fine. (When replying here I hadn't looked at the full patch yet, so please accept my apologies if this is properly explained in the description.) Jan _______________________________________________ Xen-devel mailing list Xenemail@example.com https://lists.xenproject.org/mailman/listinfo/xen-devel