On Tue, 23 Oct 2012 11:51:49 -0200
Eduardo Habkost <ehabk...@redhat.com> wrote:
> On Tue, Oct 23, 2012 at 03:26:40PM +0200, Igor Mammedov wrote:
> > convert x86_cpu_list() to use QDEV_FIND_PROP_FROM_BIT() for getting
> > CPUID feature name.
> > In addition since x86_cpu_list() was the last user of *feature_name
> > arrays, clean them up.
> >
> > Signed-off-by: Igor Mammedov <imamm...@redhat.com>
> > ---
> > v2:
> > * fix x86_cpu_list(), use X86CPU as base to count offset correctly
> > * Do not forget to print 0 bit
> > ---
> > target-i386/cpu.c | 106
> > +++++++++++-------------------------------------------
> > 1 file changed, 20 insertions(+), 86 deletions(-)
> >
> > diff --git a/target-i386/cpu.c b/target-i386/cpu.c
> > index 5e6b77d..ff29cfa 100644
> > --- a/target-i386/cpu.c
> > +++ b/target-i386/cpu.c
> > @@ -38,55 +38,6 @@
> > #include <linux/kvm_para.h>
> > #endif
> >
> > -/* feature flags taken from "Intel Processor Identification and the CPUID
> > - * Instruction" and AMD's "CPUID Specification". In cases of disagreement
> > - * between feature naming conventions, aliases may be added.
> > - */
> > -static const char *feature_name[] = {
> > - "fpu", "vme", "de", "pse",
> > - "tsc", "msr", "pae", "mce",
> > - "cx8", "apic", NULL, "sep",
> > - "mtrr", "pge", "mca", "cmov",
> > - "pat", "pse36", "pn" /* Intel psn */, "clflush" /* Intel clfsh */,
> > - NULL, "ds" /* Intel dts */, "acpi", "mmx",
> > - "fxsr", "sse", "sse2", "ss",
> > - "ht" /* Intel htt */, "tm", "ia64", "pbe",
> > -};
> > -static const char *ext_feature_name[] = {
> > - "pni|sse3" /* Intel,AMD sse3 */, "pclmulqdq|pclmuldq", "dtes64",
> > "monitor",
> > - "ds_cpl", "vmx", "smx", "est",
> > - "tm2", "ssse3", "cid", NULL,
> > - "fma", "cx16", "xtpr", "pdcm",
> > - NULL, "pcid", "dca", "sse4.1|sse4_1",
> > - "sse4.2|sse4_2", "x2apic", "movbe", "popcnt",
> > - "tsc-deadline", "aes", "xsave", "osxsave",
> > - "avx", NULL, NULL, "hypervisor",
> > -};
> > -/* Feature names that are already defined on feature_name[] but are set on
> > - * CPUID[8000_0001].EDX on AMD CPUs don't have their names on
> > - * ext2_feature_name[]. They are copied automatically to
> > cpuid_ext2_features
> > - * if and only if CPU vendor is AMD.
> > - */
> > -static const char *ext2_feature_name[] = {
> > - NULL /* fpu */, NULL /* vme */, NULL /* de */, NULL /* pse */,
> > - NULL /* tsc */, NULL /* msr */, NULL /* pae */, NULL /* mce */,
> > - NULL /* cx8 */ /* AMD CMPXCHG8B */, NULL /* apic */, NULL, "syscall",
> > - NULL /* mtrr */, NULL /* pge */, NULL /* mca */, NULL /* cmov */,
> > - NULL /* pat */, NULL /* pse36 */, NULL, NULL /* Linux mp */,
> > - "nx|xd", NULL, "mmxext", NULL /* mmx */,
> > - NULL /* fxsr */, "fxsr_opt|ffxsr", "pdpe1gb" /* AMD Page1GB */,
> > "rdtscp",
> > -};
> > -static const char *ext3_feature_name[] = {
> > - "lahf_lm" /* AMD LahfSahf */, "cmp_legacy", "svm", "extapic" /* AMD
> > ExtApicSpace */,
> > - "cr8legacy" /* AMD AltMovCr8 */, "abm", "sse4a", "misalignsse",
> > - "3dnowprefetch", "osvw", "ibs", "xop",
> > - "skinit", "wdt", NULL, NULL,
> > - "fma4", NULL, "cvt16", "nodeid_msr",
> > - NULL, NULL, NULL, NULL,
> > - NULL, NULL, NULL, NULL,
> > - NULL, NULL, NULL, NULL,
> > -};
> > -
> > #if defined(CONFIG_KVM)
> > static void x86_cpu_get_kvmclock(Object *obj, Visitor *v, void *opaque,
> > const char *name, Error **errp)
> > @@ -1495,35 +1446,22 @@ error:
> > return -1;
> > }
> >
> > -/* generate a composite string into buf of all cpuid names in featureset
> > - * selected by fbits. indicate truncation at bufsize in the event of
> > overflow.
> > - * if flags, suppress names undefined in featureset.
> > - */
> > -static void listflags(char *buf, int bufsize, uint32_t fbits,
> > - const char **featureset, uint32_t flags)
> > -{
> > - const char **p = &featureset[31];
> > - char *q, *b, bit;
> > - int nc;
> > -
> > - b = 4 <= bufsize ? buf + (bufsize -= 3) - 1 : NULL;
> > - *buf = '\0';
> > - for (q = buf, bit = 31; fbits && bufsize; --p, fbits &= ~(1 << bit),
> > --bit)
> > - if (fbits & 1 << bit && (*p || !flags)) {
> > - if (*p)
> > - nc = snprintf(q, bufsize, "%s%s", q == buf ? "" : " ", *p);
> > - else
> > - nc = snprintf(q, bufsize, "%s[%d]", q == buf ? "" : " ",
> > bit);
> > - if (bufsize <= nc) {
> > - if (b) {
> > - memcpy(b, "...", sizeof("..."));
> > - }
> > - return;
> > - }
> > - q += nc;
> > - bufsize -= nc;
> > - }
> > -}
> > +#define LIST_FLAGS(_typename, _state, _field) \
> > + do { \
> > + int i; \
> > + const Property *prop; \
> > + const DeviceClass *dc; \
> > + dc = DEVICE_CLASS(object_class_by_name((_typename))); \
> > + (*cpu_fprintf)(f, " "); \
> > + for (i = 31; i >= 0; --i) { \
> > + prop = QDEV_FIND_PROP_FROM_BIT(dc, _state, _field, i); \
> > + if (prop) { \
> > + /* for compatibility do not print f- prefix */ \
> > + (*cpu_fprintf)(f, " %s", prop->name + 2); \
> > + } \
> > + } \
> > + (*cpu_fprintf)(f, "\n"); \
> > + } while (0)
> >
> > /* generate CPU information. */
> > void x86_cpu_list(FILE *f, fprintf_function cpu_fprintf)
> > @@ -1539,14 +1477,10 @@ void x86_cpu_list(FILE *f, fprintf_function
> > cpu_fprintf)
> > (*cpu_fprintf)(f, "x86 %16s\n", "[host]");
> > }
> > (*cpu_fprintf)(f, "\nRecognized CPUID flags:\n");
> > - listflags(buf, sizeof(buf), (uint32_t)~0, feature_name, 1);
> > - (*cpu_fprintf)(f, " %s\n", buf);
> > - listflags(buf, sizeof(buf), (uint32_t)~0, ext_feature_name, 1);
> > - (*cpu_fprintf)(f, " %s\n", buf);
> > - listflags(buf, sizeof(buf), (uint32_t)~0, ext2_feature_name, 1);
> > - (*cpu_fprintf)(f, " %s\n", buf);
> > - listflags(buf, sizeof(buf), (uint32_t)~0, ext3_feature_name, 1);
> > - (*cpu_fprintf)(f, " %s\n", buf);
> > + LIST_FLAGS(TYPE_X86_CPU, X86CPU, env.cpuid_features);
> > + LIST_FLAGS(TYPE_X86_CPU, X86CPU, env.cpuid_ext_features);
> > + LIST_FLAGS(TYPE_X86_CPU, X86CPU, env.cpuid_ext2_features);
> > + LIST_FLAGS(TYPE_X86_CPU, X86CPU, env.cpuid_ext3_features);
>
> Do we really need to filter the property list and list only the ones
> that correspond to feature bits in the X86CPU struct?
>
> I mean: the specific X86CPU struct field where the CPU property bit is
> being stored isn't part of the interface. For external code, all that
> matters are the property names/types, the struct fields/offsets are
> internal implementation details.
>
> IMO we can simply list all recognized X86CPU properties, or (if listing
> all the properties is too much) list all the CPU properties that start
> with "f-".
Probably "f-" will be better because it won't surprise users who expect only
CPUID feature flags that was so far printed here.
I'll do it for the next respin.
>
> --
> Eduardo
>
--
Regards,
Igor
