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. If you actually want to notice going down the wrong path, then you want a BUG. > 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. ~Andrew _______________________________________________ Xen-devel mailing list Xenfirstname.lastname@example.org https://lists.xenproject.org/mailman/listinfo/xen-devel