[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: [Qemu-devel] [PATCH v7 2/9] i386: Add cache information in X86CPUDef
From: |
Moger, Babu |
Subject: |
Re: [Qemu-devel] [PATCH v7 2/9] i386: Add cache information in X86CPUDefinition |
Date: |
Mon, 7 May 2018 22:56:13 +0000 |
> -----Original Message-----
> From: Eduardo Habkost [mailto:address@hidden
> Sent: Monday, May 7, 2018 2:10 PM
> To: Moger, Babu <address@hidden>
> Cc: address@hidden; address@hidden; address@hidden;
> address@hidden; address@hidden; address@hidden;
> address@hidden; address@hidden; address@hidden
> Subject: Re: [Qemu-devel] [PATCH v7 2/9] i386: Add cache information in
> X86CPUDefinition
>
> On Thu, Apr 26, 2018 at 11:26:42AM -0500, Babu Moger wrote:
> > Add cache information in X86CPUDefinition and CPUX86State.
> >
> > Signed-off-by: Babu Moger <address@hidden>
> > Tested-by: Geoffrey McRae <address@hidden>
> > ---
> > target/i386/cpu.c | 4 ++++
> > target/i386/cpu.h | 8 ++++++++
> > 2 files changed, 12 insertions(+)
> >
> > diff --git a/target/i386/cpu.c b/target/i386/cpu.c
> > index b6c1592..a518a0f 100644
> > --- a/target/i386/cpu.c
> > +++ b/target/i386/cpu.c
> > @@ -1105,6 +1105,7 @@ struct X86CPUDefinition {
> > int stepping;
> > FeatureWordArray features;
> > const char *model_id;
> > + CPUCaches cache_info;
> > };
> >
> > static X86CPUDefinition builtin_x86_defs[] = {
> > @@ -3242,6 +3243,9 @@ static void x86_cpu_load_def(X86CPU *cpu,
> X86CPUDefinition *def, Error **errp)
> > env->features[w] = def->features[w];
> > }
> >
> > + /* Load Cache information from the X86CPUDefinition */
> > + memcpy(&env->cache_info, &def->cache_info, sizeof(CPUCaches));
> > +
> > /* Special cases not set in the X86CPUDefinition structs: */
> > /* TODO: in-kernel irqchip for hvf */
> > if (kvm_enabled()) {
> > diff --git a/target/i386/cpu.h b/target/i386/cpu.h
> > index fa03e2c..1213bb7 100644
> > --- a/target/i386/cpu.h
> > +++ b/target/i386/cpu.h
> > @@ -1096,6 +1096,13 @@ typedef struct CPUCacheInfo {
> > } CPUCacheInfo;
> >
> >
> > +typedef struct CPUCaches {
> > + bool valid;
> > + CPUCacheInfo l1d_cache;
> > + CPUCacheInfo l1i_cache;
> > + CPUCacheInfo l2_cache;
> > + CPUCacheInfo l3_cache;
> > +} CPUCaches;
> >
> > typedef struct CPUX86State {
> > /* standard registers */
> > @@ -1282,6 +1289,7 @@ typedef struct CPUX86State {
> > /* Features that were explicitly enabled/disabled */
> > FeatureWordArray user_features;
> > uint32_t cpuid_model[12];
> > + CPUCaches cache_info;
>
> Suggestion for a follow-up patch, or in case there's need to
> respin this series: what about making both
> X86CPUDefinition::cache_info and CPUX86State::cache_info pointers
> instead of embedded structs? This way you won't need the 'valid'
> field (you can just check if the pointer is NULL), and won't need
> the memcpy() above.
Sure. We can do that. Basically, initialize a static cache_info structure and
use the address wherever applicable.
Will try it.
>
> This shouldn't block the series, though:
>
> Reviewed-by: Eduardo Habkost <address@hidden>
>
> --
> Eduardo