[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: [Qemu-devel] [PATCH v3 1/3] x86: Data structure changes to support M
From: |
Robert Hoo |
Subject: |
Re: [Qemu-devel] [PATCH v3 1/3] x86: Data structure changes to support MSR based features |
Date: |
Sat, 18 Aug 2018 11:10:16 +0800 |
On Fri, 2018-08-17 at 00:10 -0300, Eduardo Habkost wrote:
[trim...]
> > +
> > typedef struct FeatureWordInfo {
> > - /* feature flags names are taken from "Intel Processor Identification
> > and
> > + FeatureWordType type;
> > + /* feature flags names are 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.
> > */
> > const char *feat_names[32];
> > - uint32_t cpuid_eax; /* Input EAX for CPUID */
> > - bool cpuid_needs_ecx; /* CPUID instruction uses ECX as input */
> > - uint32_t cpuid_ecx; /* Input ECX value for CPUID */
> > - int cpuid_reg; /* output register (R_* constant) */
> > + union {
> > + /* If type==CPUID_FEATURE_WORD */
> > + struct {
> > + uint32_t eax; /* Input EAX for CPUID */
> > + bool needs_ecx; /* CPUID instruction uses ECX as input */
> > + uint32_t ecx; /* Input ECX value for CPUID */
> > + int reg; /* output register (R_* constant) */
> > + } cpuid;
> > + /* If type==MSR_FEATURE_WORD */
> > + struct {
> > + uint32_t index;
> > + struct { /*CPUID that enumerate this MSR*/
> > + FeatureWord cpuid_class;
> > + uint32_t cpuid_flag;
> > + } cpuid_dep;
> > + } msr;
> > + };
> > uint32_t tcg_features; /* Feature flags supported by TCG */
> > uint32_t unmigratable_flags; /* Feature flags known to be unmigratable
> > */
> > uint32_t migratable_flags; /* Feature flags known to be migratable */
>
> The data structure change looks good, but you are breaking the
> build if you touch them without updating the existing code. If
> you break the build in your series, you break git-bisect.
>
I had tested in my environment before send out, build has no even a
warning. Is it because this patch is based on master + previous icelake
cpu model set? I see you are pulling in previous icelake cpu model patch
set. I will rebase this patch to then master. Will previous icelake cpu
model patch set appear in master soon?
>
> [...]
> > + /*Below are MSR exposed features*/
> > + [FEATURE_WORDS_ARCH_CAPABILITIES] = {
> > + .type = MSR_FEATURE_WORD,
> > + .feat_names = {
> > + "rdctl-no", "ibrs-all", "rsba", NULL,
> > + "ssb-no", NULL, NULL, NULL,
> > + NULL, NULL, NULL, NULL,
> > + NULL, NULL, NULL, NULL,
> > + NULL, NULL, NULL, NULL,
> > + NULL, NULL, NULL, NULL,
> > + NULL, NULL, NULL, NULL,
> > + NULL, NULL, NULL, NULL,
> > + },
> > + .msr = { .index = MSR_IA32_ARCH_CAPABILITIES,
> > + .cpuid_dep = { FEAT_7_0_EDX,
> > + CPUID_7_0_EDX_ARCH_CAPABILITIES }
> > + },
> > + },
>
> I suggest moving this hunk to a separate patch: first we refactor
> the existing code without changing behavior or adding new
> features, then we add the new features.
>
Sure.
> > };
> >
> > typedef struct X86RegisterInfo32 {
> > diff --git a/target/i386/cpu.h b/target/i386/cpu.h
> > index cddf9d9..9e8879e 100644
> > --- a/target/i386/cpu.h
> > +++ b/target/i386/cpu.h
> > @@ -502,9 +502,14 @@ typedef enum FeatureWord {
> > FEAT_6_EAX, /* CPUID[6].EAX */
> > FEAT_XSAVE_COMP_LO, /* CPUID[EAX=0xd,ECX=0].EAX */
> > FEAT_XSAVE_COMP_HI, /* CPUID[EAX=0xd,ECX=0].EDX */
> > + FEATURE_WORDS_NUM_CPUID,
> > + FEATURE_WORDS_FIRST_MSR = FEATURE_WORDS_NUM_CPUID,
> > + FEATURE_WORDS_ARCH_CAPABILITIES = FEATURE_WORDS_FIRST_MSR,
> > FEATURE_WORDS,
> > } FeatureWord;
> >
> > +#define FEATURE_WORDS_NUM_MSRS (FEATURE_WORDS - FEATURE_WORDS_FIRST_MSR)
> > +
> > typedef uint32_t FeatureWordArray[FEATURE_WORDS];
> >
> > /* cpuid_features bits */
> > --
> > 1.8.3.1
> >
> >
>
- [Qemu-devel] [PATCH v3 0/3] x86: QEMU side support on MSR based features, Robert Hoo, 2018/08/10
- Re: [Qemu-devel] [PATCH v3 1/3] x86: Data structure changes to support MSR based features, Paolo Bonzini, 2018/08/17
- Re: [Qemu-devel] [PATCH v3 1/3] x86: Data structure changes to support MSR based features, Eduardo Habkost, 2018/08/17
- Re: [Qemu-devel] [PATCH v3 1/3] x86: Data structure changes to support MSR based features, Paolo Bonzini, 2018/08/17
- [Qemu-devel] X86CPU "feature-words" property on QEMU (was Re: [PATCH v3 1/3] x86: Data structure changes to support MSR based) features, Eduardo Habkost, 2018/08/17
- Re: [Qemu-devel] X86CPU "feature-words" property on QEMU (was Re: [PATCH v3 1/3] x86: Data structure changes to support MSR based) features, Paolo Bonzini, 2018/08/17
- Re: [Qemu-devel] X86CPU "feature-words" property on QEMU (was Re: [PATCH v3 1/3] x86: Data structure changes to support MSR based) features, Eduardo Habkost, 2018/08/17
[Qemu-devel] [PATCH v3 2/3] kvm: Add support to KVM_GET_MSR_FEATURE_INDEX_LIST and KVM_GET_MSRS system ioctl, Robert Hoo, 2018/08/10