| Message ID | 20240925084239.85649-7-roger.pau@citrix.com (mailing list archive) |
|---|---|
| State | Superseded |
| Headers | show |
| Series | xen/livepatch: improvements to loading | expand |
On 25/09/2024 9:42 am, Roger Pau Monne wrote: > diff --git a/xen/arch/x86/include/asm/alternative.h b/xen/arch/x86/include/asm/alternative.h > index 69555d781ef9..b7f155994b2c 100644 > --- a/xen/arch/x86/include/asm/alternative.h > +++ b/xen/arch/x86/include/asm/alternative.h > @@ -7,6 +7,7 @@ > #include <xen/lib.h> > #include <xen/stringify.h> > #include <asm/asm-macros.h> > +#include <asm/cpufeatureset.h> > > struct __packed alt_instr { > int32_t orig_offset; /* original instruction */ > @@ -59,6 +60,9 @@ extern void alternative_branches(void); > alt_repl_len(n2)) "-" alt_orig_len) > > #define ALTINSTR_ENTRY(feature, num) \ > + " .if " __stringify(feature) " >= " __stringify(NCAPINTS * 32) "\n"\ Please use STR() from macros.h. It's shorter, and we're trying to retire the use of __stringify(). Happy to fix on commit. Reviewed-by: Andrew Cooper <andrew.cooper3@citrix.com>
On 25.09.2024 10:42, Roger Pau Monne wrote: > Ensure at build time the feature(s) used for the alternative blocks are in > range of the featureset. > > No functional change intended, as all current usages are correct. > > Signed-off-by: Roger Pau Monné <roger.pau@citrix.com> I'm struggling with what this is meant to guard against. All validly usable constants are within range. Any unsuitable constant can of course have any value, yet you'd then refuse only those which are out of bounds. Jan
On Wed, Sep 25, 2024 at 11:04:45AM +0200, Jan Beulich wrote: > On 25.09.2024 10:42, Roger Pau Monne wrote: > > Ensure at build time the feature(s) used for the alternative blocks are in > > range of the featureset. > > > > No functional change intended, as all current usages are correct. > > > > Signed-off-by: Roger Pau Monné <roger.pau@citrix.com> > > I'm struggling with what this is meant to guard against. All validly usable > constants are within range. Any unsuitable constant can of course have any > value, yet you'd then refuse only those which are out of bounds. It's IMO better than nothing, and it's a build-time check. Thanks, Roger.
On Wed, Sep 25, 2024 at 09:51:36AM +0100, Andrew Cooper wrote: > On 25/09/2024 9:42 am, Roger Pau Monne wrote: > > diff --git a/xen/arch/x86/include/asm/alternative.h b/xen/arch/x86/include/asm/alternative.h > > index 69555d781ef9..b7f155994b2c 100644 > > --- a/xen/arch/x86/include/asm/alternative.h > > +++ b/xen/arch/x86/include/asm/alternative.h > > @@ -7,6 +7,7 @@ > > #include <xen/lib.h> > > #include <xen/stringify.h> > > #include <asm/asm-macros.h> > > +#include <asm/cpufeatureset.h> > > > > struct __packed alt_instr { > > int32_t orig_offset; /* original instruction */ > > @@ -59,6 +60,9 @@ extern void alternative_branches(void); > > alt_repl_len(n2)) "-" alt_orig_len) > > > > #define ALTINSTR_ENTRY(feature, num) \ > > + " .if " __stringify(feature) " >= " __stringify(NCAPINTS * 32) "\n"\ > > Please use STR() from macros.h. It's shorter, and we're trying to > retire the use of __stringify(). Oh, yes, indeed. I just copied from the surrounding context and forgot about STR(). > Happy to fix on commit. Reviewed-by: Andrew Cooper > <andrew.cooper3@citrix.com> Sure, thanks. Roger.
diff --git a/xen/arch/x86/include/asm/alternative-asm.h b/xen/arch/x86/include/asm/alternative-asm.h index 4092f5ba70a6..83e8594f0eaf 100644 --- a/xen/arch/x86/include/asm/alternative-asm.h +++ b/xen/arch/x86/include/asm/alternative-asm.h @@ -12,6 +12,9 @@ * instruction. See apply_alternatives(). */ .macro altinstruction_entry orig, repl, feature, orig_len, repl_len, pad_len + .if \feature >= NCAPINTS * 32 + .error "alternative feature outside of featureset range" + .endif .long \orig - . .long \repl - . .word \feature diff --git a/xen/arch/x86/include/asm/alternative.h b/xen/arch/x86/include/asm/alternative.h index 69555d781ef9..b7f155994b2c 100644 --- a/xen/arch/x86/include/asm/alternative.h +++ b/xen/arch/x86/include/asm/alternative.h @@ -7,6 +7,7 @@ #include <xen/lib.h> #include <xen/stringify.h> #include <asm/asm-macros.h> +#include <asm/cpufeatureset.h> struct __packed alt_instr { int32_t orig_offset; /* original instruction */ @@ -59,6 +60,9 @@ extern void alternative_branches(void); alt_repl_len(n2)) "-" alt_orig_len) #define ALTINSTR_ENTRY(feature, num) \ + " .if " __stringify(feature) " >= " __stringify(NCAPINTS * 32) "\n"\ + " .error \"alternative feature outside of featureset range\"\n" \ + " .endif\n" \ " .long .LXEN%=_orig_s - .\n" /* label */ \ " .long " alt_repl_s(num)" - .\n" /* new instruction */ \ " .word " __stringify(feature) "\n" /* feature bit */ \
Ensure at build time the feature(s) used for the alternative blocks are in range of the featureset. No functional change intended, as all current usages are correct. Signed-off-by: Roger Pau Monné <roger.pau@citrix.com> --- Changes since v1: - New in this version. --- xen/arch/x86/include/asm/alternative-asm.h | 3 +++ xen/arch/x86/include/asm/alternative.h | 4 ++++ 2 files changed, 7 insertions(+)