Hi, Sorry for not reviewing the previous versions of this series (making it miss soft freeze). On Mon, Mar 12, 2018 at 05:00:45PM -0400, Babu Moger wrote: > Generalize some of the macro definitions which are generic cache > properties that are common between CPUID 4 and CPUID 0x8000001D > in preparation for adding support for 0x8000001D. > > Signed-off-by: Babu Moger <babu.moger@xxxxxxx> > --- > target/i386/cpu.c | 52 ++++++++++++++++++++++++++-------------------------- > 1 file changed, 26 insertions(+), 26 deletions(-) > > diff --git a/target/i386/cpu.c b/target/i386/cpu.c > index b5e431e..42dd381 100644 > --- a/target/i386/cpu.c > +++ b/target/i386/cpu.c > @@ -66,22 +66,22 @@ > #define CPUID_2_L3_16MB_16WAY_64B 0x4d > > > -/* CPUID Leaf 4 constants: */ > +/* Cache specific constants: */ We have too many CPUID leaves that describe caches, so I would like to explicitly document on which CPUID leaves these macros can be used. I suggest: /* Macros for CPUID[4] and CPUID[0x8000001D] */ > > /* EAX: */ > -#define CPUID_4_TYPE_DCACHE 1 > -#define CPUID_4_TYPE_ICACHE 2 > -#define CPUID_4_TYPE_UNIFIED 3 > +#define TYPE_DCACHE 1 > +#define TYPE_ICACHE 2 > +#define TYPE_UNIFIED 3 TYPE_* can be confused with QOM type names, I'd use something else. Maybe CACHE_TYPE_D, CACHE_TYPE_I, CACHE_TYPE_UNIFIED? > > -#define CPUID_4_LEVEL(l) ((l) << 5) > +#define CACHE_LEVEL(l) ((l) << 5) > > -#define CPUID_4_SELF_INIT_LEVEL (1 << 8) > -#define CPUID_4_FULLY_ASSOC (1 << 9) > +#define CACHE_SELF_INIT_LEVEL (1 << 8) > +#define CACHE_FULLY_ASSOC (1 << 9) > > /* EDX: */ > -#define CPUID_4_NO_INVD_SHARING (1 << 0) > -#define CPUID_4_INCLUSIVE (1 << 1) > -#define CPUID_4_COMPLEX_IDX (1 << 2) > +#define CACHE_NO_INVD_SHARING (1 << 0) > +#define CACHE_INCLUSIVE (1 << 1) > +#define CACHE_COMPLEX_IDX (1 << 2) > > #define ASSOC_FULL 0xFF > [...] -- Eduardo