在 2013-01-18五的 14:40 +0100，Igor Mammedov写道：
> 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.
>
No, "_vendor_words2str" is OK, though I still prefer "_vendor_str"
stubbornly :)
> 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?
>
that's true.
Thanks!
> >
> > > #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 */
> >
>
--
regards!
li guang
