| Message ID | 1454679743-18133-9-git-send-email-andrew.cooper3@citrix.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
>>> On 05.02.16 at 14:42, <andrew.cooper3@citrix.com> wrote: > --- /dev/null > +++ b/xen/arch/x86/cpuid.c > @@ -0,0 +1,19 @@ > +#include <xen/lib.h> > +#include <asm/cpuid.h> > + > +const uint32_t known_features[] = INIT_KNOWN_FEATURES; > + > +static void __maybe_unused build_assertions(void) > +{ > + BUILD_BUG_ON(ARRAY_SIZE(known_features) != FSCAPINTS); This is sort of redundant with ... > --- /dev/null > +++ b/xen/include/asm-x86/cpuid.h > @@ -0,0 +1,24 @@ > +#ifndef __X86_CPUID_H__ > +#define __X86_CPUID_H__ > + > +#include <asm/cpuid-autogen.h> > + > +#define FSCAPINTS FEATURESET_NR_ENTRIES > + > +#ifndef __ASSEMBLY__ > +#include <xen/types.h> > + > +extern const uint32_t known_features[FSCAPINTS]; ... the use of FSCAPINTS here. You'd catch more mistakes if you just used [] here. But either way Acked-by: Jan Beulich <jbeulich@suse.com> Jan
On 12/02/16 16:43, Jan Beulich wrote: >>>> On 05.02.16 at 14:42, <andrew.cooper3@citrix.com> wrote: >> --- /dev/null >> +++ b/xen/arch/x86/cpuid.c >> @@ -0,0 +1,19 @@ >> +#include <xen/lib.h> >> +#include <asm/cpuid.h> >> + >> +const uint32_t known_features[] = INIT_KNOWN_FEATURES; >> + >> +static void __maybe_unused build_assertions(void) >> +{ >> + BUILD_BUG_ON(ARRAY_SIZE(known_features) != FSCAPINTS); > This is sort of redundant with ... > >> --- /dev/null >> +++ b/xen/include/asm-x86/cpuid.h >> @@ -0,0 +1,24 @@ >> +#ifndef __X86_CPUID_H__ >> +#define __X86_CPUID_H__ >> + >> +#include <asm/cpuid-autogen.h> >> + >> +#define FSCAPINTS FEATURESET_NR_ENTRIES >> + >> +#ifndef __ASSEMBLY__ >> +#include <xen/types.h> >> + >> +extern const uint32_t known_features[FSCAPINTS]; > ... the use of FSCAPINTS here. You'd catch more mistakes if you > just used [] here. Not quite. The extern gives an explicit size so other translation units can use ARRAY_SIZE(). Without the BUILD_BUG_ON(), const uint32_t known_features[] can actually be longer than FSCAPINTS, and everything compiles fine. The BUILD_BUG_ON() were introduced following an off-by-one error generating INIT_KNOWN_FEATURES, where ARRAY_SIZE(known_features) was different in this translation unit than all others. > > But either way > Acked-by: Jan Beulich <jbeulich@suse.com> Thanks, ~Andrew
>>> On 12.02.16 at 17:48, <andrew.cooper3@citrix.com> wrote: > On 12/02/16 16:43, Jan Beulich wrote: >>>>> On 05.02.16 at 14:42, <andrew.cooper3@citrix.com> wrote: >>> --- /dev/null >>> +++ b/xen/arch/x86/cpuid.c >>> @@ -0,0 +1,19 @@ >>> +#include <xen/lib.h> >>> +#include <asm/cpuid.h> >>> + >>> +const uint32_t known_features[] = INIT_KNOWN_FEATURES; >>> + >>> +static void __maybe_unused build_assertions(void) >>> +{ >>> + BUILD_BUG_ON(ARRAY_SIZE(known_features) != FSCAPINTS); >> This is sort of redundant with ... >> >>> --- /dev/null >>> +++ b/xen/include/asm-x86/cpuid.h >>> @@ -0,0 +1,24 @@ >>> +#ifndef __X86_CPUID_H__ >>> +#define __X86_CPUID_H__ >>> + >>> +#include <asm/cpuid-autogen.h> >>> + >>> +#define FSCAPINTS FEATURESET_NR_ENTRIES >>> + >>> +#ifndef __ASSEMBLY__ >>> +#include <xen/types.h> >>> + >>> +extern const uint32_t known_features[FSCAPINTS]; >> ... the use of FSCAPINTS here. You'd catch more mistakes if you >> just used [] here. > > Not quite. > > The extern gives an explicit size so other translation units can use > ARRAY_SIZE(). True. > Without the BUILD_BUG_ON(), const uint32_t known_features[] can actually > be longer than FSCAPINTS, and everything compiles fine. > > The BUILD_BUG_ON() were introduced following an off-by-one error > generating INIT_KNOWN_FEATURES, where ARRAY_SIZE(known_features) was > different in this translation unit than all others. But what if INIT_KNOWN_FEATURES inits fewer than the intended number of elements. The remaining array members will be zero, sure, but I think such a condition would suggest a mistake elsewhere, and hence might be worth flagging. Jan
On 12/02/16 17:14, Jan Beulich wrote: >>>> On 12.02.16 at 17:48, <andrew.cooper3@citrix.com> wrote: >> On 12/02/16 16:43, Jan Beulich wrote: >>>>>> On 05.02.16 at 14:42, <andrew.cooper3@citrix.com> wrote: >>>> --- /dev/null >>>> +++ b/xen/arch/x86/cpuid.c >>>> @@ -0,0 +1,19 @@ >>>> +#include <xen/lib.h> >>>> +#include <asm/cpuid.h> >>>> + >>>> +const uint32_t known_features[] = INIT_KNOWN_FEATURES; >>>> + >>>> +static void __maybe_unused build_assertions(void) >>>> +{ >>>> + BUILD_BUG_ON(ARRAY_SIZE(known_features) != FSCAPINTS); >>> This is sort of redundant with ... >>> >>>> --- /dev/null >>>> +++ b/xen/include/asm-x86/cpuid.h >>>> @@ -0,0 +1,24 @@ >>>> +#ifndef __X86_CPUID_H__ >>>> +#define __X86_CPUID_H__ >>>> + >>>> +#include <asm/cpuid-autogen.h> >>>> + >>>> +#define FSCAPINTS FEATURESET_NR_ENTRIES >>>> + >>>> +#ifndef __ASSEMBLY__ >>>> +#include <xen/types.h> >>>> + >>>> +extern const uint32_t known_features[FSCAPINTS]; >>> ... the use of FSCAPINTS here. You'd catch more mistakes if you >>> just used [] here. >> Not quite. >> >> The extern gives an explicit size so other translation units can use >> ARRAY_SIZE(). > True. > >> Without the BUILD_BUG_ON(), const uint32_t known_features[] can actually >> be longer than FSCAPINTS, and everything compiles fine. >> >> The BUILD_BUG_ON() were introduced following an off-by-one error >> generating INIT_KNOWN_FEATURES, where ARRAY_SIZE(known_features) was >> different in this translation unit than all others. > But what if INIT_KNOWN_FEATURES inits fewer than the intended > number of elements. The remaining array members will be zero, sure, > but I think such a condition would suggest a mistake elsewhere, and > hence might be worth flagging. In principle, implicit zero extending is ok. In practice, the autogen script explicitly zero extends the identifier to the intended number of words. ~Andrew
diff --git a/xen/arch/x86/Makefile b/xen/arch/x86/Makefile index 8e6e901..0e2b1d5 100644 --- a/xen/arch/x86/Makefile +++ b/xen/arch/x86/Makefile @@ -12,6 +12,7 @@ obj-y += bitops.o obj-bin-y += bzimage.init.o obj-bin-y += clear_page.o obj-bin-y += copy_page.o +obj-y += cpuid.o obj-y += compat.o x86_64/compat.o obj-$(CONFIG_KEXEC) += crash.o obj-y += debug.o diff --git a/xen/arch/x86/cpu/common.c b/xen/arch/x86/cpu/common.c index a99cc7c..151dfe4 100644 --- a/xen/arch/x86/cpu/common.c +++ b/xen/arch/x86/cpu/common.c @@ -341,6 +341,9 @@ void identify_cpu(struct cpuinfo_x86 *c) * The vendor-specific functions might have changed features. Now * we do "generic changes." */ + for (i = 0; i < FSCAPINTS; ++i) { + c->x86_capability[i] &= known_features[i]; + } for (i = 0 ; i < NCAPINTS ; ++i) c->x86_capability[i] &= ~cleared_caps[i]; diff --git a/xen/arch/x86/cpuid.c b/xen/arch/x86/cpuid.c new file mode 100644 index 0000000..fb3a6ac --- /dev/null +++ b/xen/arch/x86/cpuid.c @@ -0,0 +1,19 @@ +#include <xen/lib.h> +#include <asm/cpuid.h> + +const uint32_t known_features[] = INIT_KNOWN_FEATURES; + +static void __maybe_unused build_assertions(void) +{ + BUILD_BUG_ON(ARRAY_SIZE(known_features) != FSCAPINTS); +} + +/* + * Local variables: + * mode: C + * c-file-style: "BSD" + * c-basic-offset: 4 + * tab-width: 4 + * indent-tabs-mode: nil + * End: + */ diff --git a/xen/include/asm-x86/cpufeature.h b/xen/include/asm-x86/cpufeature.h index d069563..a984a81 100644 --- a/xen/include/asm-x86/cpufeature.h +++ b/xen/include/asm-x86/cpufeature.h @@ -13,9 +13,8 @@ #include <public/arch-x86/cpufeatureset.h> -#include <asm/cpuid-autogen.h> +#include <asm/cpuid.h> -#define FSCAPINTS FEATURESET_NR_ENTRIES #define NCAPINTS (FSCAPINTS + 1) /* N 32-bit words worth of info */ /* Other features, Linux-defined mapping, FSMAX+1 */ diff --git a/xen/include/asm-x86/cpuid.h b/xen/include/asm-x86/cpuid.h new file mode 100644 index 0000000..6cca5ea --- /dev/null +++ b/xen/include/asm-x86/cpuid.h @@ -0,0 +1,24 @@ +#ifndef __X86_CPUID_H__ +#define __X86_CPUID_H__ + +#include <asm/cpuid-autogen.h> + +#define FSCAPINTS FEATURESET_NR_ENTRIES + +#ifndef __ASSEMBLY__ +#include <xen/types.h> + +extern const uint32_t known_features[FSCAPINTS]; + +#endif /* __ASSEMBLY__ */ +#endif /* !__X86_CPUID_H__ */ + +/* + * Local variables: + * mode: C + * c-file-style: "BSD" + * c-basic-offset: 4 + * tab-width: 4 + * indent-tabs-mode: nil + * End: + */ diff --git a/xen/tools/gen-cpuid.py b/xen/tools/gen-cpuid.py index c8240c0..0843be6 100755 --- a/xen/tools/gen-cpuid.py +++ b/xen/tools/gen-cpuid.py @@ -19,6 +19,8 @@ class State(object): # State calculated self.nr_entries = 0 # Number of words in a featureset + self.common = 0 # Common features between 1d and e1d + self.known = [] # All known features def parse_definitions(state): """ @@ -89,6 +91,22 @@ def crunch_numbers(state): # Size of bitmaps state.nr_entries = nr_entries = (max(state.names.keys()) >> 5) + 1 + # Features common between 1d and e1d. + common_1d = (FPU, VME, DE, PSE, TSC, MSR, PAE, MCE, CX8, APIC, + MTRR, PGE, MCA, CMOV, PAT, PSE36, MMX, FXSR) + + # All known features. Duplicate the common features in e1d + e1d_base = (SYSCALL >> 5) << 5 + state.known = featureset_to_uint32s( + state.names.keys() + [ e1d_base + (x % 32) for x in common_1d ], + nr_entries) + + # Fold common back into names + for f in common_1d: + state.names[e1d_base + (f % 32)] = "E1D_" + state.names[f] + + state.common = featureset_to_uint32s(common_1d, 1)[0] + def write_results(state): state.output.write( @@ -103,7 +121,13 @@ def write_results(state): state.output.write( """ #define FEATURESET_NR_ENTRIES %s + +#define INIT_COMMON_FEATURES %s + +#define INIT_KNOWN_FEATURES { \\\n%s\n} """ % (state.nr_entries, + state.common, + format_uint32s(state.known, 4), )) state.output.write(
If Xen doesn't know about a feature, it is unsafe for use and should be deliberately hidden from Xen's capabilities. This doesn't make a practical difference yet, but will make a difference later when the guest featuresets are seeded from the host featureset. Signed-off-by: Andrew Cooper <andrew.cooper3@citrix.com> --- CC: Jan Beulich <JBeulich@suse.com> v2: * Reduced substantially from v1, by using the autogenerated information. --- xen/arch/x86/Makefile | 1 + xen/arch/x86/cpu/common.c | 3 +++ xen/arch/x86/cpuid.c | 19 +++++++++++++++++++ xen/include/asm-x86/cpufeature.h | 3 +-- xen/include/asm-x86/cpuid.h | 24 ++++++++++++++++++++++++ xen/tools/gen-cpuid.py | 24 ++++++++++++++++++++++++ 6 files changed, 72 insertions(+), 2 deletions(-) create mode 100644 xen/arch/x86/cpuid.c create mode 100644 xen/include/asm-x86/cpuid.h