search for: apply_alternatives

...oeuf wrote: > > On Tue, Oct 17, 2017 at 11:36:57AM -0400, Boris Ostrovsky wrote: > >> On 10/17/2017 10:36 AM, Josh Poimboeuf wrote: > >>> Maybe we can add a new field to the alternatives entry struct which > >>> specifies the offset to the CALL instruction, so apply_alternatives() > >>> can find it. > >> We'd also have to assume that the restore part of an alternative entry > >> is the same size as the save part. Which is true now. > > Why? > > > > Don't you need to know the size of the instruction without save and &...

...oeuf wrote: > > On Tue, Oct 17, 2017 at 11:36:57AM -0400, Boris Ostrovsky wrote: > >> On 10/17/2017 10:36 AM, Josh Poimboeuf wrote: > >>> Maybe we can add a new field to the alternatives entry struct which > >>> specifies the offset to the CALL instruction, so apply_alternatives() > >>> can find it. > >> We'd also have to assume that the restore part of an alternative entry > >> is the same size as the save part. Which is true now. > > Why? > > > > Don't you need to know the size of the instruction without save and &...

...ions with calls to pv ops. > > + */ > > +void __init apply_pv_alternatives(void) > > +{ > > + setup_force_cpu_cap(X86_FEATURE_PV_OPS); > > Not for Xen HVM guests. >From what I can tell, HVM guests still use pv_time_ops and pv_mmu_ops.exit_mmap, right? > > + apply_alternatives(__pv_alt_instructions, __pv_alt_instructions_end); > > +} > > > This is a problem (at least for Xen PV guests): > apply_alternatives()->text_poke_early()->local_irq_save()->...'cli'->death. Ah, right. > It might be possible not to turn off/on the interru...

...ions with calls to pv ops. > > + */ > > +void __init apply_pv_alternatives(void) > > +{ > > + setup_force_cpu_cap(X86_FEATURE_PV_OPS); > > Not for Xen HVM guests. >From what I can tell, HVM guests still use pv_time_ops and pv_mmu_ops.exit_mmap, right? > > + apply_alternatives(__pv_alt_instructions, __pv_alt_instructions_end); > > +} > > > This is a problem (at least for Xen PV guests): > apply_alternatives()->text_poke_early()->local_irq_save()->...'cli'->death. Ah, right. > It might be possible not to turn off/on the interru...

...> >> The displacement that the linker computes will be relative to the where > >> this instruction is placed at the time of linking, which is in > >> .pv_altinstructions (and not .text). So when we copy it into .text the > >> displacement becomes bogus. > > apply_alternatives() is supposed to adjust that displacement based on > > the new IP, though it could be messing that up somehow. (See patch > > 10/13.) > > > > That patch doesn't take into account the fact that replacement > instructions may have to save/restore registers. So, for ex...

...> >> The displacement that the linker computes will be relative to the where > >> this instruction is placed at the time of linking, which is in > >> .pv_altinstructions (and not .text). So when we copy it into .text the > >> displacement becomes bogus. > > apply_alternatives() is supposed to adjust that displacement based on > > the new IP, though it could be messing that up somehow. (See patch > > 10/13.) > > > > That patch doesn't take into account the fact that replacement > instructions may have to save/restore registers. So, for ex...

On Tue, Oct 17, 2017 at 11:36:57AM -0400, Boris Ostrovsky wrote: > On 10/17/2017 10:36 AM, Josh Poimboeuf wrote: > > > > Maybe we can add a new field to the alternatives entry struct which > > specifies the offset to the CALL instruction, so apply_alternatives() > > can find it. > > We'd also have to assume that the restore part of an alternative entry > is the same size as the save part. Which is true now. Why? -- Josh

On Tue, Oct 17, 2017 at 11:36:57AM -0400, Boris Ostrovsky wrote: > On 10/17/2017 10:36 AM, Josh Poimboeuf wrote: > > > > Maybe we can add a new field to the alternatives entry struct which > > specifies the offset to the CALL instruction, so apply_alternatives() > > can find it. > > We'd also have to assume that the restore part of an alternative entry > is the same size as the save part. Which is true now. Why? -- Josh

...x86/include/asm/alternative.h b/arch/x86/include/asm/alternative.h index c096624137ae..8482f90d5078 100644 --- a/arch/x86/include/asm/alternative.h +++ b/arch/x86/include/asm/alternative.h @@ -61,6 +61,7 @@ extern int alternatives_patched; extern void alternative_instructions(void); extern void apply_alternatives(struct alt_instr *start, struct alt_instr *end); +extern void apply_pv_alternatives(void); struct module; @@ -132,14 +133,17 @@ static inline int alternatives_text_reserved(void *start, void *end) b_replacement(num)":\n\t" newinstr "\n" e_replacement(num) ":\n\t&quot...

...gt;> +void __init apply_pv_alternatives(void) >>>> +{ >>>> + setup_force_cpu_cap(X86_FEATURE_PV_OPS); >>> Not for Xen HVM guests. >> From what I can tell, HVM guests still use pv_time_ops and >> pv_mmu_ops.exit_mmap, right? >> >>>> + apply_alternatives(__pv_alt_instructions, __pv_alt_instructions_end); >>>> +} >>> This is a problem (at least for Xen PV guests): >>> apply_alternatives()->text_poke_early()->local_irq_save()->...'cli'->death. >> Ah, right. >> >>> It might be poss...

...gt;> +void __init apply_pv_alternatives(void) >>>> +{ >>>> + setup_force_cpu_cap(X86_FEATURE_PV_OPS); >>> Not for Xen HVM guests. >> From what I can tell, HVM guests still use pv_time_ops and >> pv_mmu_ops.exit_mmap, right? >> >>>> + apply_alternatives(__pv_alt_instructions, __pv_alt_instructions_end); >>>> +} >>> This is a problem (at least for Xen PV guests): >>> apply_alternatives()->text_poke_early()->local_irq_save()->...'cli'->death. >> Ah, right. >> >>> It might be poss...

...an normal alternatives. > + * They are only applied when running on a hypervisor. They replace some > + * native instructions with calls to pv ops. > + */ > +void __init apply_pv_alternatives(void) > +{ > + setup_force_cpu_cap(X86_FEATURE_PV_OPS); Not for Xen HVM guests. > + apply_alternatives(__pv_alt_instructions, __pv_alt_instructions_end); > +} This is a problem (at least for Xen PV guests): apply_alternatives()->text_poke_early()->local_irq_save()->...'cli'->death. It might be possible not to turn off/on the interrupts in this particular case since the gues...

...>>> On Tue, Oct 17, 2017 at 11:36:57AM -0400, Boris Ostrovsky wrote: >>>> On 10/17/2017 10:36 AM, Josh Poimboeuf wrote: >>>>> Maybe we can add a new field to the alternatives entry struct which >>>>> specifies the offset to the CALL instruction, so apply_alternatives() >>>>> can find it. >>>> We'd also have to assume that the restore part of an alternative entry >>>> is the same size as the save part. Which is true now. >>> Why? >>> >> Don't you need to know the size of the instruction witho...

Hi Andi, Here's a little batch of cleanups: - re-enable VDSO when PARAVIRT is enabled - make the parainstructions symbols match the other altinstructions naming convention - add kernel command-line options to disable altinstructions for smp and pv_ops Oh, and I'm mailing your noreplacement patch back at you, for no particularly good reason. J --

Hi Andi, Here's a little batch of cleanups: - re-enable VDSO when PARAVIRT is enabled - make the parainstructions symbols match the other altinstructions naming convention - add kernel command-line options to disable altinstructions for smp and pv_ops Oh, and I'm mailing your noreplacement patch back at you, for no particularly good reason. J --

...ops. >>> + */ >>> +void __init apply_pv_alternatives(void) >>> +{ >>> + setup_force_cpu_cap(X86_FEATURE_PV_OPS); >> Not for Xen HVM guests. > From what I can tell, HVM guests still use pv_time_ops and > pv_mmu_ops.exit_mmap, right? > >>> + apply_alternatives(__pv_alt_instructions, __pv_alt_instructions_end); >>> +} >> >> This is a problem (at least for Xen PV guests): >> apply_alternatives()->text_poke_early()->local_irq_save()->...'cli'->death. > Ah, right. > >> It might be possible not to tur...

...ne PV_INDIRECT(addr) *addr(%rip) > > The displacement that the linker computes will be relative to the where > this instruction is placed at the time of linking, which is in > .pv_altinstructions (and not .text). So when we copy it into .text the > displacement becomes bogus. apply_alternatives() is supposed to adjust that displacement based on the new IP, though it could be messing that up somehow. (See patch 10/13.) -- Josh

...+------- 1 file changed, 15 insertions(+), 7 deletions(-) diff --git a/arch/x86/kernel/alternative.c b/arch/x86/kernel/alternative.c index 3344d3382e91..81c577c7deba 100644 --- a/arch/x86/kernel/alternative.c +++ b/arch/x86/kernel/alternative.c @@ -410,20 +410,28 @@ void __init_or_module noinline apply_alternatives(struct alt_instr *start, insnbuf_sz = a->replacementlen; /* - * 0xe8 is a relative jump; fix the offset. - * - * Instruction length is checked before the opcode to avoid - * accessing uninitialized bytes for zero-length replacements. + * Fix the address offsets for call and jum...

...v_alternatives(void) >>>>> +{ >>>>> + setup_force_cpu_cap(X86_FEATURE_PV_OPS); >>>> Not for Xen HVM guests. >>> From what I can tell, HVM guests still use pv_time_ops and >>> pv_mmu_ops.exit_mmap, right? >>> >>>>> + apply_alternatives(__pv_alt_instructions, __pv_alt_instructions_end); >>>>> +} >>>> This is a problem (at least for Xen PV guests): >>>> apply_alternatives()->text_poke_early()->local_irq_save()->...'cli'->death. >>> Ah, right. >>> >>&...

...gt; > > > diff --git a/arch/x86/kernel/alternative.c b/arch/x86/kernel/alternative.c > > index 3344d3382e91..81c577c7deba 100644 > > --- a/arch/x86/kernel/alternative.c > > +++ b/arch/x86/kernel/alternative.c > > @@ -410,20 +410,28 @@ void __init_or_module noinline apply_alternatives(struct alt_instr *start, > > insnbuf_sz = a->replacementlen; > > > > /* > > - * 0xe8 is a relative jump; fix the offset. > > - * > > - * Instruction length is checked before the opcode to avoid > > - * accessing uninitialized bytes for ze...