| Message ID | 20210309134813.23912-5-jgross@suse.com (mailing list archive) |
|---|---|
| State | Superseded |
| Headers | show |
| Series | x86: major paravirt cleanup | expand |
On Tue, Mar 09, 2021 at 02:48:05PM +0100, Juergen Gross wrote: > Add support for alternative patching for the case a feature is not > present on the current cpu. > > For users of ALTERNATIVE() and friends an inverted feature is specified > by applying the ALT_NOT() macro to it, e.g.: > > ALTERNATIVE(old, new, ALT_NOT(feature)) > > Signed-off-by: Juergen Gross <jgross@suse.com> > --- > V5: > - split off from next patch > - reworked to use flag byte (Boris Petkov) > V6: > - rework again to not use flag byte (Boris Petkov) > --- > arch/x86/include/asm/alternative-asm.h | 3 +++ > arch/x86/include/asm/alternative.h | 3 +++ > arch/x86/kernel/alternative.c | 19 ++++++++++++++----- > 3 files changed, 20 insertions(+), 5 deletions(-) LGTM, minor touchups I'd do ontop: --- diff --git a/arch/x86/include/asm/alternative.h b/arch/x86/include/asm/alternative.h index 89889618ae01..fd205cdcfbad 100644 --- a/arch/x86/include/asm/alternative.h +++ b/arch/x86/include/asm/alternative.h @@ -55,18 +55,18 @@ ".long 999b - .\n\t" \ ".popsection\n\t" +#define ALTINSTR_FLAG_INV (1 << 15) +#define ALT_NOT(feat) ((feat) | ALTINSTR_FLAG_INV) + struct alt_instr { s32 instr_offset; /* original instruction */ s32 repl_offset; /* offset to replacement instruction */ u16 cpuid; /* cpuid bit set for replacement */ -#define ALTINSTR_FLAG_INV (1 << 15) u8 instrlen; /* length of original instruction */ u8 replacementlen; /* length of new instruction */ u8 padlen; /* length of build-time padding */ } __packed; -#define ALT_NOT(feat) ((feat) | ALTINSTR_FLAG_INV) - /* * Debug flag that can be tested to see whether alternative * instructions were patched in already: diff --git a/arch/x86/kernel/alternative.c b/arch/x86/kernel/alternative.c index d8e669a1546f..133b549dc091 100644 --- a/arch/x86/kernel/alternative.c +++ b/arch/x86/kernel/alternative.c @@ -397,9 +397,10 @@ void __init_or_module noinline apply_alternatives(struct alt_instr *start, BUG_ON(feature >= (NCAPINTS + NBUGINTS) * 32); /* - * Drop out if either: - * - feature not available, but required, or - * - feature available, but NOT required + * Patch if either: + * - feature is present + * - feature not present but ALTINSTR_FLAG_INV is set to mean, + * patch if feature is *NOT* present. */ if (!boot_cpu_has(feature) == !(a->cpuid & ALTINSTR_FLAG_INV)) { if (a->padlen > 1)
On 10.03.21 07:07, Borislav Petkov wrote: > On Tue, Mar 09, 2021 at 02:48:05PM +0100, Juergen Gross wrote: >> Add support for alternative patching for the case a feature is not >> present on the current cpu. >> >> For users of ALTERNATIVE() and friends an inverted feature is specified >> by applying the ALT_NOT() macro to it, e.g.: >> >> ALTERNATIVE(old, new, ALT_NOT(feature)) >> >> Signed-off-by: Juergen Gross <jgross@suse.com> >> --- >> V5: >> - split off from next patch >> - reworked to use flag byte (Boris Petkov) >> V6: >> - rework again to not use flag byte (Boris Petkov) >> --- >> arch/x86/include/asm/alternative-asm.h | 3 +++ >> arch/x86/include/asm/alternative.h | 3 +++ >> arch/x86/kernel/alternative.c | 19 ++++++++++++++----- >> 3 files changed, 20 insertions(+), 5 deletions(-) > > LGTM, minor touchups I'd do ontop: > > --- > > diff --git a/arch/x86/include/asm/alternative.h b/arch/x86/include/asm/alternative.h > index 89889618ae01..fd205cdcfbad 100644 > --- a/arch/x86/include/asm/alternative.h > +++ b/arch/x86/include/asm/alternative.h > @@ -55,18 +55,18 @@ > ".long 999b - .\n\t" \ > ".popsection\n\t" > > +#define ALTINSTR_FLAG_INV (1 << 15) > +#define ALT_NOT(feat) ((feat) | ALTINSTR_FLAG_INV) > + > struct alt_instr { > s32 instr_offset; /* original instruction */ > s32 repl_offset; /* offset to replacement instruction */ > u16 cpuid; /* cpuid bit set for replacement */ > -#define ALTINSTR_FLAG_INV (1 << 15) > u8 instrlen; /* length of original instruction */ > u8 replacementlen; /* length of new instruction */ > u8 padlen; /* length of build-time padding */ > } __packed; > > -#define ALT_NOT(feat) ((feat) | ALTINSTR_FLAG_INV) > - Did you look at patch 13? :-) > /* > * Debug flag that can be tested to see whether alternative > * instructions were patched in already: > diff --git a/arch/x86/kernel/alternative.c b/arch/x86/kernel/alternative.c > index d8e669a1546f..133b549dc091 100644 > --- a/arch/x86/kernel/alternative.c > +++ b/arch/x86/kernel/alternative.c > @@ -397,9 +397,10 @@ void __init_or_module noinline apply_alternatives(struct alt_instr *start, > BUG_ON(feature >= (NCAPINTS + NBUGINTS) * 32); > > /* > - * Drop out if either: > - * - feature not available, but required, or > - * - feature available, but NOT required > + * Patch if either: > + * - feature is present > + * - feature not present but ALTINSTR_FLAG_INV is set to mean, > + * patch if feature is *NOT* present. Okay. Juergen
On Wed, Mar 10, 2021 at 08:52:40AM +0100, Jürgen Groß wrote:
> Did you look at patch 13? :-)
Well, I usually review in increasing patch order. :-P
But make that change here pls because otherwise unnecessary churn.
Thx.
On 10.03.21 15:15, Borislav Petkov wrote: > On Wed, Mar 10, 2021 at 08:52:40AM +0100, Jürgen Groß wrote: >> Did you look at patch 13? :-) > > Well, I usually review in increasing patch order. :-P > > But make that change here pls because otherwise unnecessary churn. Okay. Juergen
diff --git a/arch/x86/include/asm/alternative-asm.h b/arch/x86/include/asm/alternative-asm.h index 464034db299f..3965daf0460e 100644 --- a/arch/x86/include/asm/alternative-asm.h +++ b/arch/x86/include/asm/alternative-asm.h @@ -6,6 +6,9 @@ #include <asm/asm.h> +#define ALTINSTR_FLAG_INV (1 << 15) +#define ALT_NOT(feat) ((feat) | ALTINSTR_FLAG_INV) + #ifdef CONFIG_SMP .macro LOCK_PREFIX 672: lock diff --git a/arch/x86/include/asm/alternative.h b/arch/x86/include/asm/alternative.h index 5753fb2ac489..89889618ae01 100644 --- a/arch/x86/include/asm/alternative.h +++ b/arch/x86/include/asm/alternative.h @@ -59,11 +59,14 @@ struct alt_instr { s32 instr_offset; /* original instruction */ s32 repl_offset; /* offset to replacement instruction */ u16 cpuid; /* cpuid bit set for replacement */ +#define ALTINSTR_FLAG_INV (1 << 15) u8 instrlen; /* length of original instruction */ u8 replacementlen; /* length of new instruction */ u8 padlen; /* length of build-time padding */ } __packed; +#define ALT_NOT(feat) ((feat) | ALTINSTR_FLAG_INV) + /* * Debug flag that can be tested to see whether alternative * instructions were patched in already: diff --git a/arch/x86/kernel/alternative.c b/arch/x86/kernel/alternative.c index 8d778e46725d..d8e669a1546f 100644 --- a/arch/x86/kernel/alternative.c +++ b/arch/x86/kernel/alternative.c @@ -388,21 +388,30 @@ void __init_or_module noinline apply_alternatives(struct alt_instr *start, */ for (a = start; a < end; a++) { int insn_buff_sz = 0; + /* Mask away "NOT" flag bit for feature to test. */ + u16 feature = a->cpuid & ~ALTINSTR_FLAG_INV; instr = (u8 *)&a->instr_offset + a->instr_offset; replacement = (u8 *)&a->repl_offset + a->repl_offset; BUG_ON(a->instrlen > sizeof(insn_buff)); - BUG_ON(a->cpuid >= (NCAPINTS + NBUGINTS) * 32); - if (!boot_cpu_has(a->cpuid)) { + BUG_ON(feature >= (NCAPINTS + NBUGINTS) * 32); + + /* + * Drop out if either: + * - feature not available, but required, or + * - feature available, but NOT required + */ + if (!boot_cpu_has(feature) == !(a->cpuid & ALTINSTR_FLAG_INV)) { if (a->padlen > 1) optimize_nops(a, instr); continue; } - DPRINTK("feat: %d*32+%d, old: (%pS (%px) len: %d), repl: (%px, len: %d), pad: %d", - a->cpuid >> 5, - a->cpuid & 0x1f, + DPRINTK("feat: %s%d*32+%d, old: (%pS (%px) len: %d), repl: (%px, len: %d), pad: %d", + (a->cpuid & ALTINSTR_FLAG_INV) ? "!" : "", + feature >> 5, + feature & 0x1f, instr, instr, a->instrlen, replacement, a->replacementlen, a->padlen);
Add support for alternative patching for the case a feature is not present on the current cpu. For users of ALTERNATIVE() and friends an inverted feature is specified by applying the ALT_NOT() macro to it, e.g.: ALTERNATIVE(old, new, ALT_NOT(feature)) Signed-off-by: Juergen Gross <jgross@suse.com> --- V5: - split off from next patch - reworked to use flag byte (Boris Petkov) V6: - rework again to not use flag byte (Boris Petkov) --- arch/x86/include/asm/alternative-asm.h | 3 +++ arch/x86/include/asm/alternative.h | 3 +++ arch/x86/kernel/alternative.c | 19 ++++++++++++++----- 3 files changed, 20 insertions(+), 5 deletions(-)