On Fri, 18 Jan 2013 15:12:36 +0800
li guang <lig.f...@cn.fujitsu.com> wrote:
> 在 2013-01-17四的 16:16 +0100，Igor Mammedov写道：
>
> >
> > diff --git a/target-i386/cpu.c b/target-i386/cpu.c
> > index ce914da..ab80dbe 100644
> > --- a/target-i386/cpu.c
> > +++ b/target-i386/cpu.c
> > @@ -45,6 +45,18 @@
> > #include "hw/apic_internal.h"
> > #endif
> >
> > +static void x86_cpu_vendor_words2str(char *dst, uint32_t vendor1,
> > + uint32_t vendor2, uint32_t vendor3)
>
> sorry, but I should say "_vendor_words2str" seems not so suitable,
> it's mostly not a convertor, but a compactor, so I suggest to use
> "_vendor_str" directly.
I think that "_vendor_words2str" describes more clearly what function does,
regardless whether it is conversion or compaction. "_vendor_str" seems more
ambiguous though. But if you insist, I can change to it.
BTW: it's not just copying, it copies from little endinan words to string.
>
> > +{
> > + int i;
> > + for (i = 0; i < 4; i++) {
> > + dst[i] = vendor1 >> (8 * i);
> > + dst[i + 4] = vendor2 >> (8 * i);
> > + dst[i + 8] = vendor3 >> (8 * i);
> > + }
> > + dst[CPUID_VENDOR_SZ] = '\0';
> > +}
> > +
>
> > --- a/target-i386/cpu.h
> > +++ b/target-i386/cpu.h
> > @@ -537,14 +537,14 @@ typedef uint32_t FeatureWordArray[FEATURE_WORDS];
> > #define CPUID_VENDOR_INTEL_1 0x756e6547 /* "Genu" */
> > #define CPUID_VENDOR_INTEL_2 0x49656e69 /* "ineI" */
> > #define CPUID_VENDOR_INTEL_3 0x6c65746e /* "ntel" */
> > +#define CPUID_VENDOR_INTEL "GenuineIntel"
> >
>
> you said the reason you did not remove _VENDOR_INTEL_{1,2,3}
> is they're used somewhere, did you mean "target-i386/translate.c"
> for sysenter instruction?
> if it is, why can't we also remove them there?
That would imply conversion of CPUX86State to using string for cpuid_vendor
instead of currents words which would mean to do conversion every time cpuid
instruction is called in guest. I'd rather keep current cpuid_vendor{1,2,3}
in CPUX86State.
Purpose of this patch is to switch from direct field copying when initializing
CPU to using property setter.
If we ever decide to convert CPUX86State.cpuid_vendor{1,2,3} into string, it
could be done by a separate patch.
In addition, wouldn't strcmp() there be less effective performance wise,
versus just number comparison if we would convert
CPUX86State.cpuid_vendor{1,2,3} to string?
>
> > #define CPUID_VENDOR_AMD_1 0x68747541 /* "Auth" */
> > #define CPUID_VENDOR_AMD_2 0x69746e65 /* "enti" */
> > #define CPUID_VENDOR_AMD_3 0x444d4163 /* "cAMD" */
> > +#define CPUID_VENDOR_AMD "AuthenticAMD"
> >
> > -#define CPUID_VENDOR_VIA_1 0x746e6543 /* "Cent" */
> > -#define CPUID_VENDOR_VIA_2 0x48727561 /* "aurH" */
> > -#define CPUID_VENDOR_VIA_3 0x736c7561 /* "auls" */
> > +#define CPUID_VENDOR_VIA "CentaurHauls"
> >
> > #define CPUID_MWAIT_IBE (1 << 1) /* Interrupts can exit capability */
> > #define CPUID_MWAIT_EMX (1 << 0) /* enumeration supported */
>
